Make existing and new issue URLs configurable (#10710)

* Make existing and new issue URLs configurable

* Apply a deep merge over sdk config to allow sane nested structures

* Defaultize

* Fix types

* Iterate

* Add FeedbackDialog snapshot test

* Add SdkConfig snapshot tests

* Iterate

* Fix tests

* Iterate types

* Fix test
pull/28217/head
Michael Telatynski 2023-04-26 10:36:00 +01:00 committed by GitHub
parent e4610e4672
commit 6166dbb661
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 259 additions and 78 deletions

View File

@ -54,3 +54,25 @@ export type KeysStartingWith<Input extends object, Str extends string> = {
}[keyof Input];
export type NonEmptyArray<T> = [T, ...T[]];
export type Defaultize<P, D> = P extends any
? string extends keyof P
? P
: Pick<P, Exclude<keyof P, keyof D>> &
Partial<Pick<P, Extract<keyof P, keyof D>>> &
Partial<Pick<D, Exclude<keyof D, keyof P>>>
: never;
export type DeepReadonly<T> = T extends (infer R)[]
? DeepReadonlyArray<R>
: T extends Function
? T
: T extends object
? DeepReadonlyObject<T>
: T;
interface DeepReadonlyArray<T> extends ReadonlyArray<DeepReadonly<T>> {}
type DeepReadonlyObject<T> = {
readonly [P in keyof T]: DeepReadonly<T[P]>;
};

View File

@ -49,6 +49,7 @@ import ActiveWidgetStore from "../stores/ActiveWidgetStore";
import AutoRageshakeStore from "../stores/AutoRageshakeStore";
import { IConfigOptions } from "../IConfigOptions";
import { MatrixDispatcher } from "../dispatcher/dispatcher";
import { DeepReadonly } from "./common";
/* eslint-disable @typescript-eslint/naming-convention */
@ -59,7 +60,7 @@ declare global {
Olm: {
init: () => Promise<void>;
};
mxReactSdkConfig: IConfigOptions;
mxReactSdkConfig: DeepReadonly<IConfigOptions>;
// Needed for Safari, unknown to TypeScript
webkitAudioContext: typeof AudioContext;

View File

@ -186,6 +186,11 @@ export interface IConfigOptions {
description: string;
show_once?: boolean;
};
feedback: {
existing_issues_url: string;
new_issue_url: string;
};
}
export interface ISsoRedirectOptions {

View File

@ -23,6 +23,7 @@ import { TypedEventEmitter } from "matrix-js-sdk/src/models/typed-event-emitter"
import dis from "./dispatcher/dispatcher";
import AsyncWrapper from "./AsyncWrapper";
import { Defaultize } from "./@types/common";
const DIALOG_CONTAINER_ID = "mx_Dialog_Container";
const STATIC_DIALOG_CONTAINER_ID = "mx_Dialog_StaticContainer";
@ -32,14 +33,6 @@ export type ComponentType = React.ComponentType<{
onFinished?(...args: any): void;
}>;
type Defaultize<P, D> = P extends any
? string extends keyof P
? P
: Pick<P, Exclude<keyof P, keyof D>> &
Partial<Pick<P, Extract<keyof P, keyof D>>> &
Partial<Pick<D, Exclude<keyof D, keyof P>>>
: never;
// Generic type which returns the props of the Modal component with the onFinished being optional.
export type ComponentProps<C extends ComponentType> = Defaultize<
Omit<React.ComponentProps<C>, "onFinished">,

View File

@ -16,12 +16,15 @@ limitations under the License.
*/
import { Optional } from "matrix-events-sdk";
import { mergeWith } from "lodash";
import { SnakedObject } from "./utils/SnakedObject";
import { IConfigOptions, ISsoRedirectOptions } from "./IConfigOptions";
import { isObject, objectClone } from "./utils/objects";
import { DeepReadonly, Defaultize } from "./@types/common";
// see element-web config.md for docs, or the IConfigOptions interface for dev docs
export const DEFAULTS: IConfigOptions = {
export const DEFAULTS: DeepReadonly<IConfigOptions> = {
brand: "Element",
integrations_ui_url: "https://scalar.vector.im/",
integrations_rest_url: "https://scalar.vector.im/api",
@ -50,13 +53,43 @@ export const DEFAULTS: IConfigOptions = {
chunk_length: 2 * 60, // two minutes
max_length: 4 * 60 * 60, // four hours
},
feedback: {
existing_issues_url:
"https://github.com/vector-im/element-web/issues?q=is%3Aopen+is%3Aissue+sort%3Areactions-%2B1-desc",
new_issue_url: "https://github.com/vector-im/element-web/issues/new/choose",
},
};
export default class SdkConfig {
private static instance: IConfigOptions;
private static fallback: SnakedObject<IConfigOptions>;
export type ConfigOptions = Defaultize<IConfigOptions, typeof DEFAULTS>;
private static setInstance(i: IConfigOptions): void {
function mergeConfig(
config: DeepReadonly<IConfigOptions>,
changes: DeepReadonly<Partial<IConfigOptions>>,
): DeepReadonly<IConfigOptions> {
// return { ...config, ...changes };
return mergeWith(objectClone(config), changes, (objValue, srcValue) => {
// Don't merge arrays, prefer values from newer object
if (Array.isArray(objValue)) {
return srcValue;
}
// Don't allow objects to get nulled out, this will break our types
if (isObject(objValue) && !isObject(srcValue)) {
return objValue;
}
});
}
type ObjectType<K extends keyof IConfigOptions> = IConfigOptions[K] extends object
? SnakedObject<NonNullable<IConfigOptions[K]>>
: Optional<SnakedObject<NonNullable<IConfigOptions[K]>>>;
export default class SdkConfig {
private static instance: DeepReadonly<IConfigOptions>;
private static fallback: SnakedObject<DeepReadonly<IConfigOptions>>;
private static setInstance(i: DeepReadonly<IConfigOptions>): void {
SdkConfig.instance = i;
SdkConfig.fallback = new SnakedObject(i);
@ -69,7 +102,7 @@ export default class SdkConfig {
public static get<K extends keyof IConfigOptions = never>(
key?: K,
altCaseName?: string,
): IConfigOptions | IConfigOptions[K] {
): DeepReadonly<IConfigOptions> | DeepReadonly<IConfigOptions>[K] {
if (key === undefined) {
// safe to cast as a fallback - we want to break the runtime contract in this case
return SdkConfig.instance || <IConfigOptions>{};
@ -77,32 +110,29 @@ export default class SdkConfig {
return SdkConfig.fallback.get(key, altCaseName);
}
public static getObject<K extends keyof IConfigOptions>(
key: K,
altCaseName?: string,
): Optional<SnakedObject<NonNullable<IConfigOptions[K]>>> {
public static getObject<K extends keyof IConfigOptions>(key: K, altCaseName?: string): ObjectType<K> {
const val = SdkConfig.get(key, altCaseName);
if (val !== null && val !== undefined) {
if (isObject(val)) {
return new SnakedObject(val);
}
// return the same type for sensitive callers (some want `undefined` specifically)
return val === undefined ? undefined : null;
return (val === undefined ? undefined : null) as ObjectType<K>;
}
public static put(cfg: Partial<IConfigOptions>): void {
SdkConfig.setInstance({ ...DEFAULTS, ...cfg });
public static put(cfg: DeepReadonly<ConfigOptions>): void {
SdkConfig.setInstance(mergeConfig(DEFAULTS, cfg));
}
/**
* Resets the config to be completely empty.
* Resets the config.
*/
public static unset(): void {
SdkConfig.setInstance(<IConfigOptions>{}); // safe to cast - defaults will be applied
public static reset(): void {
SdkConfig.setInstance(mergeConfig(DEFAULTS, {})); // safe to cast - defaults will be applied
}
public static add(cfg: Partial<IConfigOptions>): void {
SdkConfig.put({ ...SdkConfig.get(), ...cfg });
public static add(cfg: Partial<ConfigOptions>): void {
SdkConfig.put(mergeConfig(SdkConfig.get(), cfg));
}
}

View File

@ -66,11 +66,11 @@ import RightPanelStore from "../../stores/right-panel/RightPanelStore";
import { TimelineRenderingType } from "../../contexts/RoomContext";
import { KeyBindingAction } from "../../accessibility/KeyboardShortcuts";
import { SwitchSpacePayload } from "../../dispatcher/payloads/SwitchSpacePayload";
import { IConfigOptions } from "../../IConfigOptions";
import LeftPanelLiveShareWarning from "../views/beacon/LeftPanelLiveShareWarning";
import { UserOnboardingPage } from "../views/user-onboarding/UserOnboardingPage";
import { PipContainer } from "./PipContainer";
import { monitorSyncedPushRules } from "../../utils/pushRules/monitorSyncedPushRules";
import { ConfigOptions } from "../../SdkConfig";
// We need to fetch each pinned message individually (if we don't already have it)
// so each pinned message may trigger a request. Limit the number per room for sanity.
@ -98,7 +98,7 @@ interface IProps {
roomOobData?: IOOBData;
currentRoomId: string;
collapseLhs: boolean;
config: IConfigOptions;
config: ConfigOptions;
currentUserId?: string;
justRegistered?: boolean;
roomJustCreatedOpts?: IOpts;

View File

@ -28,10 +28,6 @@ import { submitFeedback } from "../../../rageshake/submit-rageshake";
import { useStateToggle } from "../../../hooks/useStateToggle";
import StyledCheckbox from "../elements/StyledCheckbox";
const existingIssuesUrl =
"https://github.com/vector-im/element-web/issues" + "?q=is%3Aopen+is%3Aissue+sort%3Areactions-%2B1-desc";
const newIssueUrl = "https://github.com/vector-im/element-web/issues/new/choose";
interface IProps {
feature?: string;
onFinished(): void;
@ -117,6 +113,9 @@ const FeedbackDialog: React.FC<IProps> = (props: IProps) => {
);
}
const existingIssuesUrl = SdkConfig.getObject("feedback").get("existing_issues_url");
const newIssueUrl = SdkConfig.getObject("feedback").get("new_issue_url");
return (
<QuestionDialog
className="mx_FeedbackDialog"

View File

@ -53,7 +53,7 @@ import { UPDATE_EVENT } from "../../../stores/AsyncStore";
import { isVideoRoom as calcIsVideoRoom } from "../../../utils/video-rooms";
import LegacyCallHandler, { LegacyCallHandlerEvent } from "../../../LegacyCallHandler";
import { useFeatureEnabled, useSettingValue } from "../../../hooks/useSettings";
import SdkConfig, { DEFAULTS } from "../../../SdkConfig";
import SdkConfig from "../../../SdkConfig";
import { useEventEmitterState, useTypedEventEmitterState } from "../../../hooks/useEventEmitter";
import { useWidgets } from "../right_panel/RoomSummaryCard";
import { WidgetType } from "../../../widgets/WidgetType";
@ -207,7 +207,7 @@ const VideoCallButton: FC<VideoCallButtonProps> = ({ room, busy, setBusy, behavi
let menu: JSX.Element | null = null;
if (menuOpen) {
const buttonRect = buttonRef.current!.getBoundingClientRect();
const brand = SdkConfig.get("element_call").brand ?? DEFAULTS.element_call.brand;
const brand = SdkConfig.get("element_call").brand;
menu = (
<IconizedContextMenu {...aboveLeftOf(buttonRect)} onFinished={closeMenu}>
<IconizedContextMenuOptionList>
@ -250,7 +250,7 @@ const CallButtons: FC<CallButtonsProps> = ({ room }) => {
const videoRoomsEnabled = useFeatureEnabled("feature_video_rooms");
const isVideoRoom = useMemo(() => videoRoomsEnabled && calcIsVideoRoom(room), [videoRoomsEnabled, room]);
const useElementCallExclusively = useMemo(() => {
return SdkConfig.get("element_call").use_exclusively ?? DEFAULTS.element_call.use_exclusively;
return SdkConfig.get("element_call").use_exclusively;
}, []);
const hasLegacyCall = useEventEmitterState(

View File

@ -18,6 +18,7 @@ import { MatrixClient } from "matrix-js-sdk/src/client";
import BasePlatform from "../../BasePlatform";
import { IConfigOptions } from "../../IConfigOptions";
import { DeepReadonly } from "../../@types/common";
export type DeviceClientInformation = {
name?: string;
@ -49,7 +50,7 @@ export const getClientInformationEventType = (deviceId: string): string => `${cl
*/
export const recordClientInformation = async (
matrixClient: MatrixClient,
sdkConfig: IConfigOptions,
sdkConfig: DeepReadonly<IConfigOptions>,
platform?: BasePlatform,
): Promise<void> => {
const deviceId = matrixClient.getDeviceId()!;

View File

@ -141,3 +141,12 @@ export function objectKeyChanges<O extends {}>(a: O, b: O): (keyof O)[] {
export function objectClone<O extends {}>(obj: O): O {
return JSON.parse(JSON.stringify(obj));
}
/**
* Simple object check.
* @param item
* @returns {boolean}
*/
export function isObject(item: any): item is object {
return item && typeof item === "object" && !Array.isArray(item);
}

View File

@ -305,7 +305,7 @@ describe("LegacyCallHandler", () => {
MatrixClientPeg.unset();
document.body.removeChild(audioElement);
SdkConfig.unset();
SdkConfig.reset();
});
it("should look up the correct user and start a call in the room when a phone number is dialled", async () => {
@ -516,7 +516,7 @@ describe("LegacyCallHandler without third party protocols", () => {
MatrixClientPeg.unset();
document.body.removeChild(audioElement);
SdkConfig.unset();
SdkConfig.reset();
});
it("should still start a native call", async () => {

View File

@ -77,7 +77,7 @@ describe("PosthogAnalytics", () => {
Object.defineProperty(window, "crypto", {
value: null,
});
SdkConfig.unset(); // we touch the config, so clean up
SdkConfig.reset(); // we touch the config, so clean up
});
describe("Initialisation", () => {

View File

@ -30,6 +30,9 @@ describe("SdkConfig", () => {
chunk_length: 42,
max_length: 1337,
},
feedback: {
existing_issues_url: "https://existing",
} as any,
});
});
@ -37,7 +40,16 @@ describe("SdkConfig", () => {
const customConfig = JSON.parse(JSON.stringify(DEFAULTS));
customConfig.voice_broadcast.chunk_length = 42;
customConfig.voice_broadcast.max_length = 1337;
customConfig.feedback.existing_issues_url = "https://existing";
expect(SdkConfig.get()).toEqual(customConfig);
});
it("should allow overriding individual fields of sub-objects", () => {
const feedback = SdkConfig.getObject("feedback");
expect(feedback.get("existing_issues_url")).toMatchInlineSnapshot(`"https://existing"`);
expect(feedback.get("new_issue_url")).toMatchInlineSnapshot(
`"https://github.com/vector-im/element-web/issues/new/choose"`,
);
});
});
});

View File

@ -61,7 +61,7 @@ describe("Login", function () {
afterEach(function () {
fetchMock.restore();
SdkConfig.unset(); // we touch the config, so clean up
SdkConfig.reset(); // we touch the config, so clean up
unmockPlatformPeg();
});

View File

@ -66,7 +66,7 @@ describe("Registration", function () {
afterEach(function () {
fetchMock.restore();
SdkConfig.unset(); // we touch the config, so clean up
SdkConfig.reset(); // we touch the config, so clean up
unmockPlatformPeg();
});

View File

@ -23,7 +23,7 @@ import SdkConfig from "../../../../src/SdkConfig";
describe("CountryDropdown", () => {
describe("default_country_code", () => {
afterEach(() => {
SdkConfig.unset();
SdkConfig.reset();
});
it.each([

View File

@ -0,0 +1,35 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
import React from "react";
import { render } from "@testing-library/react";
import SdkConfig from "../../../../src/SdkConfig";
import FeedbackDialog from "../../../../src/components/views/dialogs/FeedbackDialog";
describe("FeedbackDialog", () => {
it("should respect feedback config", () => {
SdkConfig.put({
feedback: {
existing_issues_url: "http://existing?foo=bar",
new_issue_url: "https://new.issue.url?foo=bar",
},
});
const { asFragment } = render(<FeedbackDialog onFinished={jest.fn()} />);
expect(asFragment()).toMatchSnapshot();
});
});

View File

@ -0,0 +1,82 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`FeedbackDialog should respect feedback config 1`] = `
<DocumentFragment>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
<div
aria-describedby="mx_Dialog_content"
aria-labelledby="mx_BaseDialog_title"
class="mx_QuestionDialog mx_FeedbackDialog mx_Dialog_fixedWidth"
data-focus-lock-disabled="false"
role="dialog"
>
<div
class="mx_Dialog_header"
>
<h2
class="mx_Heading_h2 mx_Dialog_title"
id="mx_BaseDialog_title"
>
Feedback
</h2>
</div>
<div
class="mx_Dialog_content"
id="mx_Dialog_content"
>
<div
class="mx_FeedbackDialog_section mx_FeedbackDialog_reportBug"
>
<h3>
Report a bug
</h3>
<p>
<span>
Please view
<a
href="http://existing?foo=bar"
rel="noreferrer noopener"
target="_blank"
>
existing bugs on Github
</a>
first. No match?
<a
href="https://new.issue.url?foo=bar"
rel="noreferrer noopener"
target="_blank"
>
Start a new one
</a>
.
</span>
</p>
</div>
</div>
<div
class="mx_Dialog_buttons"
>
<span
class="mx_Dialog_buttons_row"
>
<button
class="mx_Dialog_primary"
data-testid="dialog-primary-button"
type="button"
>
Go back
</button>
</span>
</div>
</div>
<div
data-focus-guard="true"
style="width: 1px; height: 0px; padding: 0px; overflow: hidden; position: fixed; top: 1px; left: 1px;"
tabindex="0"
/>
</DocumentFragment>
`;

View File

@ -120,7 +120,7 @@ describe("RoomHeader", () => {
await Promise.all([CallStore.instance, WidgetStore.instance].map(resetAsyncStoreWithClient));
client.reEmitter.stopReEmitting(room, [RoomStateEvent.Events]);
jest.restoreAllMocks();
SdkConfig.put({});
SdkConfig.reset();
});
const mockRoomType = (type: string) => {

View File

@ -42,7 +42,7 @@ async function setupTranslationOverridesForTests(overrides: ICustomTranslations)
describe("languageHandler", () => {
afterEach(() => {
SdkConfig.unset();
SdkConfig.reset();
CustomTranslationOptions.lookupFn = undefined;
});

View File

@ -20,6 +20,8 @@ import BasePlatform from "../../../src/BasePlatform";
import { IConfigOptions } from "../../../src/IConfigOptions";
import { getDeviceClientInformation, recordClientInformation } from "../../../src/utils/device/clientInformation";
import { getMockClientWithEventEmitter } from "../../test-utils";
import { DEFAULTS } from "../../../src/SdkConfig";
import { DeepReadonly } from "../../../src/@types/common";
describe("recordClientInformation()", () => {
const deviceId = "my-device-id";
@ -31,7 +33,8 @@ describe("recordClientInformation()", () => {
setAccountData: jest.fn(),
});
const sdkConfig: IConfigOptions = {
const sdkConfig: DeepReadonly<IConfigOptions> = {
...DEFAULTS,
brand: "Test Brand",
element_call: { url: "", use_exclusively: false, brand: "Element Call" },
};

View File

@ -14,24 +14,24 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import { mocked } from "jest-mock";
import SdkConfig, { DEFAULTS } from "../../../src/SdkConfig";
import SdkConfig from "../../../src/SdkConfig";
import { SettingLevel } from "../../../src/settings/SettingLevel";
import { Features } from "../../../src/settings/Settings";
import SettingsStore from "../../../src/settings/SettingsStore";
import { getChunkLength } from "../../../src/voice-broadcast/utils/getChunkLength";
jest.mock("../../../src/SdkConfig");
describe("getChunkLength", () => {
afterEach(() => {
jest.resetAllMocks();
SdkConfig.reset();
});
describe("when there is a value provided by Sdk config", () => {
beforeEach(() => {
mocked(SdkConfig.get).mockReturnValue({ chunk_length: 42 });
SdkConfig.add({
voice_broadcast: {
chunk_length: 42,
},
});
});
it("should return this value", () => {
@ -41,9 +41,11 @@ describe("getChunkLength", () => {
describe("when Sdk config does not provide a value", () => {
beforeEach(() => {
DEFAULTS.voice_broadcast = {
chunk_length: 23,
};
SdkConfig.add({
voice_broadcast: {
chunk_length: 23,
},
});
});
it("should return this value", () => {
@ -52,10 +54,6 @@ describe("getChunkLength", () => {
});
describe("when there are no defaults", () => {
beforeEach(() => {
DEFAULTS.voice_broadcast = undefined;
});
it("should return the fallback value", () => {
expect(getChunkLength()).toBe(120);
});

View File

@ -14,21 +14,21 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import { mocked } from "jest-mock";
import SdkConfig, { DEFAULTS } from "../../../src/SdkConfig";
import { getMaxBroadcastLength } from "../../../src/voice-broadcast";
jest.mock("../../../src/SdkConfig");
describe("getMaxBroadcastLength", () => {
afterEach(() => {
jest.resetAllMocks();
SdkConfig.reset();
});
describe("when there is a value provided by Sdk config", () => {
beforeEach(() => {
mocked(SdkConfig.get).mockReturnValue({ max_length: 42 });
SdkConfig.put({
voice_broadcast: {
max_length: 42,
},
});
});
it("should return this value", () => {
@ -37,23 +37,14 @@ describe("getMaxBroadcastLength", () => {
});
describe("when Sdk config does not provide a value", () => {
beforeEach(() => {
DEFAULTS.voice_broadcast = {
max_length: 23,
};
});
it("should return this value", () => {
expect(getMaxBroadcastLength()).toBe(23);
expect(getMaxBroadcastLength()).toBe(DEFAULTS.voice_broadcast!.max_length);
});
});
describe("if there are no defaults", () => {
beforeEach(() => {
DEFAULTS.voice_broadcast = undefined;
});
it("should return the fallback value", () => {
expect(DEFAULTS.voice_broadcast!.max_length).toBe(4 * 60 * 60);
expect(getMaxBroadcastLength()).toBe(4 * 60 * 60);
});
});