Split out email & phone number settings to separate components & move…

… discovery to privacy tab (#12670)

* WIP update of threepid settings section

* Remove email / phone number section from original place

and don't show the new one if 3pids are disabled

* Update snapshots

* Pull identity server / 3pid binding settings out to separate component

and put it in the security & privacy section which is its new home

* Update snapshot

* Move relevant part of test & update screenshots / snapshots

* Remove unnecessary dependency

* Add test for discovery settings

* Add spacing in terms agreement

playwright/e2e/settings/general-user-settings-tab.spec.ts

@@ -91,11 +91,6 @@ test.describe("General user settings tab", () => {
         // Assert that the default value is rendered again
         await expect(languageInput.getByText("English")).toBeVisible();
 
-        const setIdServer = uut.locator(".mx_SetIdServer");
-        await setIdServer.scrollIntoViewIfNeeded();
-        // Assert that an input area for identity server exists
-        await expect(setIdServer.getByRole("textbox", { name: "Enter a new identity server" })).toBeVisible();
-
         const setIntegrationManager = uut.locator(".mx_SetIntegrationManager");
         await setIntegrationManager.scrollIntoViewIfNeeded();
         await expect(

playwright/e2e/settings/security-user-settings-tab.spec.ts

@@ -47,5 +47,14 @@ test.describe("Security user settings tab", () => {
                 await expect(page.locator(".mx_AnalyticsLearnMoreDialog_wrapper .mx_Dialog")).toMatchScreenshot();
             });
         });
+
+        test("should contain section to set ID server", async ({ app }) => {
+            const tab = await app.settings.openUserSettings("Security");
+
+            const setIdServer = tab.locator(".mx_SetIdServer");
+            await setIdServer.scrollIntoViewIfNeeded();
+            // Assert that an input area for identity server exists
+            await expect(setIdServer.getByRole("textbox", { name: "Enter a new identity server" })).toBeVisible();
+        });
     });
 });

res/css/views/terms/_InlineTermsAgreement.pcss

@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 .mx_InlineTermsAgreement_cbContainer {
+    margin-top: var(--cpd-space-4x);
     margin-bottom: 10px;
     font: var(--cpd-font-body-md-regular);
 

src/components/views/settings/UserPersonalInfoSettings.tsx

@@ -0,0 +1,130 @@
+/*
+Copyright 2024 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, { useCallback, useEffect, useState } from "react";
+import { ThreepidMedium } from "matrix-js-sdk/src/matrix";
+import { Alert } from "@vector-im/compound-web";
+
+import AccountEmailAddresses from "./account/EmailAddresses";
+import AccountPhoneNumbers from "./account/PhoneNumbers";
+import { _t } from "../../../languageHandler";
+import InlineSpinner from "../elements/InlineSpinner";
+import SettingsSubsection from "./shared/SettingsSubsection";
+import { useMatrixClientContext } from "../../../contexts/MatrixClientContext";
+import { ThirdPartyIdentifier } from "../../../AddThreepid";
+import SettingsStore from "../../../settings/SettingsStore";
+import { UIFeature } from "../../../settings/UIFeature";
+
+type LoadingState = "loading" | "loaded" | "error";
+
+interface ThreepidSectionWrapperProps {
+    error: string;
+    loadingState: LoadingState;
+    children: React.ReactNode;
+}
+
+const ThreepidSectionWrapper: React.FC<ThreepidSectionWrapperProps> = ({ error, loadingState, children }) => {
+    if (loadingState === "loading") {
+        return <InlineSpinner />;
+    } else if (loadingState === "error") {
+        return (
+            <Alert type="critical" title={_t("common|error")}>
+                {error}
+            </Alert>
+        );
+    } else {
+        return <>{children}</>;
+    }
+};
+
+interface UserPersonalInfoSettingsProps {
+    canMake3pidChanges: boolean;
+}
+
+/**
+ * Settings controls allowing the user to set personal information like email addresses.
+ */
+export const UserPersonalInfoSettings: React.FC<UserPersonalInfoSettingsProps> = ({ canMake3pidChanges }) => {
+    const [emails, setEmails] = useState<ThirdPartyIdentifier[] | undefined>();
+    const [phoneNumbers, setPhoneNumbers] = useState<ThirdPartyIdentifier[] | undefined>();
+    const [loadingState, setLoadingState] = useState<"loading" | "loaded" | "error">("loading");
+
+    const client = useMatrixClientContext();
+
+    useEffect(() => {
+        (async () => {
+            try {
+                const threepids = await client.getThreePids();
+                setEmails(threepids.threepids.filter((a) => a.medium === ThreepidMedium.Email));
+                setPhoneNumbers(threepids.threepids.filter((a) => a.medium === ThreepidMedium.Phone));
+                setLoadingState("loaded");
+            } catch (e) {
+                setLoadingState("error");
+            }
+        })();
+    }, [client]);
+
+    const onEmailsChange = useCallback((emails: ThirdPartyIdentifier[]) => {
+        setEmails(emails);
+    }, []);
+
+    const onMsisdnsChange = useCallback((msisdns: ThirdPartyIdentifier[]) => {
+        setPhoneNumbers(msisdns);
+    }, []);
+
+    if (!SettingsStore.getValue(UIFeature.ThirdPartyID)) return null;
+
+    return (
+        <div>
+            <h2>{_t("settings|general|personal_info")}</h2>
+            <SettingsSubsection
+                heading={_t("settings|general|emails_heading")}
+                stretchContent
+                data-testid="mx_AccountEmailAddresses"
+            >
+                <ThreepidSectionWrapper
+                    error={_t("settings|general|unable_to_load_emails")}
+                    loadingState={loadingState}
+                >
+                    <AccountEmailAddresses
+                        emails={emails!}
+                        onEmailsChange={onEmailsChange}
+                        disabled={!canMake3pidChanges}
+                    />
+                </ThreepidSectionWrapper>
+            </SettingsSubsection>
+
+            <SettingsSubsection
+                heading={_t("settings|general|msisdns_heading")}
+                stretchContent
+                data-testid="mx_AccountPhoneNumbers"
+            >
+                <ThreepidSectionWrapper
+                    error={_t("settings|general|unable_to_load_msisdns")}
+                    loadingState={loadingState}
+                >
+                    <AccountPhoneNumbers
+                        msisdns={phoneNumbers!}
+                        onMsisdnsChange={onMsisdnsChange}
+                        disabled={!canMake3pidChanges}
+                    />
+                </ThreepidSectionWrapper>
+            </SettingsSubsection>
+        </div>
+    );
+};
+
+export default UserPersonalInfoSettings;

src/components/views/settings/discovery/DiscoverySettings.tsx

@@ -0,0 +1,190 @@
+/*
+Copyright 2024 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, { useCallback, useEffect, useState } from "react";
+import { SERVICE_TYPES, ThreepidMedium } from "matrix-js-sdk/src/matrix";
+import { logger } from "matrix-js-sdk/src/logger";
+import { Alert } from "@vector-im/compound-web";
+
+import DiscoveryEmailAddresses from "../discovery/EmailAddresses";
+import DiscoveryPhoneNumbers from "../discovery/PhoneNumbers";
+import { getThreepidsWithBindStatus } from "../../../../boundThreepids";
+import { useMatrixClientContext } from "../../../../contexts/MatrixClientContext";
+import { ThirdPartyIdentifier } from "../../../../AddThreepid";
+import SettingsStore from "../../../../settings/SettingsStore";
+import { UIFeature } from "../../../../settings/UIFeature";
+import { _t } from "../../../../languageHandler";
+import SetIdServer from "../SetIdServer";
+import SettingsSubsection from "../shared/SettingsSubsection";
+import InlineTermsAgreement from "../../terms/InlineTermsAgreement";
+import { Service, ServicePolicyPair, startTermsFlow } from "../../../../Terms";
+import IdentityAuthClient from "../../../../IdentityAuthClient";
+import { abbreviateUrl } from "../../../../utils/UrlUtils";
+import { useDispatcher } from "../../../../hooks/useDispatcher";
+import defaultDispatcher from "../../../../dispatcher/dispatcher";
+import { ActionPayload } from "../../../../dispatcher/payloads";
+
+type RequiredPolicyInfo =
+    | {
+          // This object is passed along to a component for handling
+          policiesAndServices: null; // From the startTermsFlow callback
+          agreedUrls: null; // From the startTermsFlow callback
+          resolve: null; // Promise resolve function for startTermsFlow callback
+      }
+    | {
+          policiesAndServices: ServicePolicyPair[];
+          agreedUrls: string[];
+          resolve: (values: string[]) => void;
+      };
+
+/**
+ * Settings controlling how a user's email addreses and phone numbers can be used to discover them
+ */
+export const DiscoverySettings: React.FC = () => {
+    const client = useMatrixClientContext();
+
+    const [emails, setEmails] = useState<ThirdPartyIdentifier[]>([]);
+    const [phoneNumbers, setPhoneNumbers] = useState<ThirdPartyIdentifier[]>([]);
+    const [loadingState, setLoadingState] = useState<"loading" | "loaded" | "error">("loading");
+    const [idServerName, setIdServerName] = useState<string | undefined>(abbreviateUrl(client.getIdentityServerUrl()));
+    const [canMake3pidChanges, setCanMake3pidChanges] = useState<boolean>(false);
+
+    const [requiredPolicyInfo, setRequiredPolicyInfo] = useState<RequiredPolicyInfo>({
+        // This object is passed along to a component for handling
+        policiesAndServices: null, // From the startTermsFlow callback
+        agreedUrls: null, // From the startTermsFlow callback
+        resolve: null, // Promise resolve function for startTermsFlow callback
+    });
+    const [hasTerms, setHasTerms] = useState<boolean>(false);
+
+    const getThreepidState = useCallback(async () => {
+        const threepids = await getThreepidsWithBindStatus(client);
+        setEmails(threepids.filter((a) => a.medium === ThreepidMedium.Email));
+        setPhoneNumbers(threepids.filter((a) => a.medium === ThreepidMedium.Phone));
+    }, [client]);
+
+    useDispatcher(
+        defaultDispatcher,
+        useCallback(
+            (payload: ActionPayload) => {
+                if (payload.action === "id_server_changed") {
+                    setIdServerName(abbreviateUrl(client.getIdentityServerUrl()));
+
+                    getThreepidState().then();
+                }
+            },
+            [client, getThreepidState],
+        ),
+    );
+
+    useEffect(() => {
+        (async () => {
+            try {
+                await getThreepidState();
+
+                const capabilities = await client.getCapabilities();
+                setCanMake3pidChanges(
+                    !capabilities["m.3pid_changes"] || capabilities["m.3pid_changes"].enabled === true,
+                );
+
+                // By starting the terms flow we get the logic for checking which terms the user has signed
+                // for free. So we might as well use that for our own purposes.
+                const idServerUrl = client.getIdentityServerUrl();
+                if (!idServerUrl) {
+                    return;
+                }
+
+                const authClient = new IdentityAuthClient();
+                try {
+                    const idAccessToken = await authClient.getAccessToken({ check: false });
+                    await startTermsFlow(
+                        client,
+                        [new Service(SERVICE_TYPES.IS, idServerUrl, idAccessToken!)],
+                        (policiesAndServices, agreedUrls, extraClassNames) => {
+                            return new Promise((resolve) => {
+                                setIdServerName(abbreviateUrl(idServerUrl));
+                                setHasTerms(true);
+                                setRequiredPolicyInfo({
+                                    policiesAndServices,
+                                    agreedUrls,
+                                    resolve,
+                                });
+                            });
+                        },
+                    );
+                    // User accepted all terms
+                    setHasTerms(false);
+                } catch (e) {
+                    logger.warn(
+                        `Unable to reach identity server at ${idServerUrl} to check ` + `for terms in Settings`,
+                    );
+                    logger.warn(e);
+                }
+
+                setLoadingState("loaded");
+            } catch (e) {
+                setLoadingState("error");
+            }
+        })();
+    }, [client, getThreepidState]);
+
+    if (!SettingsStore.getValue(UIFeature.ThirdPartyID)) return null;
+
+    if (hasTerms && requiredPolicyInfo.policiesAndServices) {
+        const intro = (
+            <Alert type="info" title={_t("settings|general|discovery_needs_terms_title")}>
+                {_t("settings|general|discovery_needs_terms", { serverName: idServerName })}
+            </Alert>
+        );
+        return (
+            <>
+                <InlineTermsAgreement
+                    policiesAndServicePairs={requiredPolicyInfo.policiesAndServices}
+                    agreedUrls={requiredPolicyInfo.agreedUrls}
+                    onFinished={requiredPolicyInfo.resolve}
+                    introElement={intro}
+                />
+                {/* has its own heading as it includes the current identity server */}
+                <SetIdServer missingTerms={true} />
+            </>
+        );
+    }
+
+    const threepidSection = idServerName ? (
+        <>
+            <DiscoveryEmailAddresses
+                emails={emails}
+                isLoading={loadingState === "loading"}
+                disabled={!canMake3pidChanges}
+            />
+            <DiscoveryPhoneNumbers
+                msisdns={phoneNumbers}
+                isLoading={loadingState === "loading"}
+                disabled={!canMake3pidChanges}
+            />
+        </>
+    ) : null;
+
+    return (
+        <SettingsSubsection heading={_t("settings|discovery|title")} data-testid="discoverySection">
+            {threepidSection}
+            {/* has its own heading as it includes the current identity server */}
+            <SetIdServer missingTerms={false} />
+        </SettingsSubsection>
+    );
+};
+
+export default DiscoverySettings;