Merge pull request #3419 from matrix-org/jryans/settings-3pid-state

Lift 3PID state management up to Settings tab

src/IdentityAuthClient.js

@@ -65,7 +65,7 @@ export default class IdentityAuthClient {
     }
 
     // Returns a promise that resolves to the access_token string from the IS
-    async getAccessToken(check=true) {
+    async getAccessToken({ check = true } = {}) {
         if (!this.authEnabled) {
             // The current IS doesn't support authentication
             return null;

src/boundThreepids.js

@@ -16,7 +16,7 @@ limitations under the License.
 
 import IdentityAuthClient from './IdentityAuthClient';
 
-export async function getThreepidBindStatus(client, filterMedium) {
+export async function getThreepidsWithBindStatus(client, filterMedium) {
     const userId = client.getUserId();
 
     let { threepids } = await client.getThreePids();
@@ -24,27 +24,33 @@ export async function getThreepidBindStatus(client, filterMedium) {
         threepids = threepids.filter((a) => a.medium === filterMedium);
     }
 
-    if (threepids.length > 0) {
+    // Check bind status assuming we have an IS and terms are agreed
-        // TODO: Handle terms agreement
+    if (threepids.length > 0 && !!client.getIdentityServerUrl()) {
-        // See https://github.com/vector-im/riot-web/issues/10522
+        try {
-        const authClient = new IdentityAuthClient();
+            const authClient = new IdentityAuthClient();
-        const identityAccessToken = await authClient.getAccessToken();
+            const identityAccessToken = await authClient.getAccessToken({ check: false });
 
-        // Restructure for lookup query
+            // Restructure for lookup query
-        const query = threepids.map(({ medium, address }) => [medium, address]);
+            const query = threepids.map(({ medium, address }) => [medium, address]);
-        const lookupResults = await client.bulkLookupThreePids(query, identityAccessToken);
+            const lookupResults = await client.bulkLookupThreePids(query, identityAccessToken);
 
-        // Record which are already bound
+            // Record which are already bound
-        for (const [medium, address, mxid] of lookupResults.threepids) {
+            for (const [medium, address, mxid] of lookupResults.threepids) {
-            if (mxid !== userId) {
+                if (mxid !== userId) {
-                continue;
+                    continue;
+                }
+                if (filterMedium && medium !== filterMedium) {
+                    continue;
+                }
+                const threepid = threepids.find(e => e.medium === medium && e.address === address);
+                if (!threepid) continue;
+                threepid.bound = true;
             }
-            if (filterMedium && medium !== filterMedium) {
+        } catch (e) {
-                continue;
+            // Ignore terms errors here and assume other flows handle this
+            if (!(e.errcode === "M_TERMS_NOT_SIGNED")) {
+                throw e;
             }
-            const threepid = threepids.find(e => e.medium === medium && e.address === address);
-            if (!threepid) continue;
-            threepid.bound = true;
         }
     }
 

src/components/views/settings/SetIdServer.js

@@ -22,7 +22,7 @@ import sdk from '../../../index';
 import MatrixClientPeg from "../../../MatrixClientPeg";
 import Modal from '../../../Modal';
 import dis from "../../../dispatcher";
-import { getThreepidBindStatus } from '../../../boundThreepids';
+import { getThreepidsWithBindStatus } from '../../../boundThreepids';
 import IdentityAuthClient from "../../../IdentityAuthClient";
 import {SERVICE_TYPES} from "matrix-js-sdk";
 import {abbreviateUrl, unabbreviateUrl} from "../../../utils/UrlUtils";
@@ -249,7 +249,7 @@ export default class SetIdServer extends React.Component {
     };
 
     async _showServerChangeWarning({ title, unboundMessage, button }) {
-        const threepids = await getThreepidBindStatus(MatrixClientPeg.get());
+        const threepids = await getThreepidsWithBindStatus(MatrixClientPeg.get());
 
         const boundThreepids = threepids.filter(tp => tp.bound);
         let message;

src/components/views/settings/account/EmailAddresses.js

@@ -23,8 +23,8 @@ import Field from "../../elements/Field";
 import AccessibleButton from "../../elements/AccessibleButton";
 import * as Email from "../../../../email";
 import AddThreepid from "../../../../AddThreepid";
-const sdk = require('../../../../index');
+import sdk from '../../../../index';
-const Modal = require("../../../../Modal");
+import Modal from '../../../../Modal';
 
 /*
 TODO: Improve the UX for everything in here.
@@ -113,11 +113,15 @@ export class ExistingEmailAddress extends React.Component {
 }
 
 export default class EmailAddresses extends React.Component {
-    constructor() {
+    static propTypes = {
-        super();
+        emails: PropTypes.array.isRequired,
+        onEmailsChange: PropTypes.func.isRequired,
+    }
+
+    constructor(props) {
+        super(props);
 
         this.state = {
-            emails: [],
             verifying: false,
             addTask: null,
             continueDisabled: false,
@@ -125,16 +129,9 @@ export default class EmailAddresses extends React.Component {
         };
     }
 
-    componentWillMount(): void {
-        const client = MatrixClientPeg.get();
-
-        client.getThreePids().then((addresses) => {
-            this.setState({emails: addresses.threepids.filter((a) => a.medium === 'email')});
-        });
-    }
-
     _onRemoved = (address) => {
-        this.setState({emails: this.state.emails.filter((e) => e !== address)});
+        const emails = this.props.emails.filter((e) => e !== address);
+        this.props.onEmailsChange(emails);
     };
 
     _onChangeNewEmailAddress = (e) => {
@@ -184,12 +181,16 @@ export default class EmailAddresses extends React.Component {
         this.state.addTask.checkEmailLinkClicked().then(() => {
             const email = this.state.newEmailAddress;
             this.setState({
-                emails: [...this.state.emails, {address: email, medium: "email"}],
                 addTask: null,
                 continueDisabled: false,
                 verifying: false,
                 newEmailAddress: "",
             });
+            const emails = [
+                ...this.props.emails,
+                { address: email, medium: "email" },
+            ];
+            this.props.onEmailsChange(emails);
         }).catch((err) => {
             this.setState({continueDisabled: false});
             if (err.errcode !== 'M_THREEPID_AUTH_FAILED') {
@@ -204,7 +205,7 @@ export default class EmailAddresses extends React.Component {
     };
 
     render() {
-        const existingEmailElements = this.state.emails.map((e) => {
+        const existingEmailElements = this.props.emails.map((e) => {
             return <ExistingEmailAddress email={e} onRemoved={this._onRemoved} key={e.address} />;
         });
 

src/components/views/settings/account/PhoneNumbers.js

@@ -23,8 +23,8 @@ import Field from "../../elements/Field";
 import AccessibleButton from "../../elements/AccessibleButton";
 import AddThreepid from "../../../../AddThreepid";
 import CountryDropdown from "../../auth/CountryDropdown";
-const sdk = require('../../../../index');
+import sdk from '../../../../index';
-const Modal = require("../../../../Modal");
+import Modal from '../../../../Modal';
 
 /*
 TODO: Improve the UX for everything in here.
@@ -108,11 +108,15 @@ export class ExistingPhoneNumber extends React.Component {
 }
 
 export default class PhoneNumbers extends React.Component {
-    constructor() {
+    static propTypes = {
-        super();
+        msisdns: PropTypes.array.isRequired,
+        onMsisdnsChange: PropTypes.func.isRequired,
+    }
+
+    constructor(props) {
+        super(props);
 
         this.state = {
-            msisdns: [],
             verifying: false,
             verifyError: false,
             verifyMsisdn: "",
@@ -124,16 +128,9 @@ export default class PhoneNumbers extends React.Component {
         };
     }
 
-    componentWillMount(): void {
-        const client = MatrixClientPeg.get();
-
-        client.getThreePids().then((addresses) => {
-            this.setState({msisdns: addresses.threepids.filter((a) => a.medium === 'msisdn')});
-        });
-    }
-
     _onRemoved = (address) => {
-        this.setState({msisdns: this.state.msisdns.filter((e) => e !== address)});
+        const msisdns = this.props.msisdns.filter((e) => e !== address);
+        this.props.onMsisdnsChange(msisdns);
     };
 
     _onChangeNewPhoneNumber = (e) => {
@@ -181,7 +178,6 @@ export default class PhoneNumbers extends React.Component {
         const token = this.state.newPhoneNumberCode;
         this.state.addTask.haveMsisdnToken(token).then(() => {
             this.setState({
-                msisdns: [...this.state.msisdns, {address: this.state.verifyMsisdn, medium: "msisdn"}],
                 addTask: null,
                 continueDisabled: false,
                 verifying: false,
@@ -190,6 +186,11 @@ export default class PhoneNumbers extends React.Component {
                 newPhoneNumber: "",
                 newPhoneNumberCode: "",
             });
+            const msisdns = [
+                ...this.props.msisdns,
+                { address: this.state.verifyMsisdn, medium: "msisdn" },
+            ];
+            this.props.onMsisdnsChange(msisdns);
         }).catch((err) => {
             this.setState({continueDisabled: false});
             if (err.errcode !== 'M_THREEPID_AUTH_FAILED') {
@@ -210,7 +211,7 @@ export default class PhoneNumbers extends React.Component {
     };
 
     render() {
-        const existingPhoneElements = this.state.msisdns.map((p) => {
+        const existingPhoneElements = this.props.msisdns.map((p) => {
             return <ExistingPhoneNumber msisdn={p} onRemoved={this._onRemoved} key={p.address} />;
         });
 

src/components/views/settings/discovery/EmailAddresses.js

@@ -23,7 +23,6 @@ import MatrixClientPeg from "../../../../MatrixClientPeg";
 import sdk from '../../../../index';
 import Modal from '../../../../Modal';
 import AddThreepid from '../../../../AddThreepid';
-import { getThreepidBindStatus } from '../../../../boundThreepids';
 
 /*
 TODO: Improve the UX for everything in here.
@@ -59,6 +58,11 @@ export class EmailAddress extends React.Component {
         };
     }
 
+    componentWillReceiveProps(nextProps) {
+        const { bound } = nextProps.email;
+        this.setState({ bound });
+    }
+
     async changeBinding({ bind, label, errorTitle }) {
         const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog");
         const { medium, address } = this.props.email;
@@ -187,27 +191,14 @@ export class EmailAddress extends React.Component {
 }
 
 export default class EmailAddresses extends React.Component {
-    constructor() {
+    static propTypes = {
-        super();
+        emails: PropTypes.array.isRequired,
-
-        this.state = {
-            loaded: false,
-            emails: [],
-        };
-    }
-
-    async componentWillMount() {
-        const client = MatrixClientPeg.get();
-
-        const emails = await getThreepidBindStatus(client, 'email');
-
-        this.setState({ emails });
     }
 
     render() {
         let content;
-        if (this.state.emails.length > 0) {
+        if (this.props.emails.length > 0) {
-            content = this.state.emails.map((e) => {
+            content = this.props.emails.map((e) => {
                 return <EmailAddress email={e} key={e.address} />;
             });
         } else {

src/components/views/settings/discovery/PhoneNumbers.js

@@ -23,7 +23,6 @@ import MatrixClientPeg from "../../../../MatrixClientPeg";
 import sdk from '../../../../index';
 import Modal from '../../../../Modal';
 import AddThreepid from '../../../../AddThreepid';
-import { getThreepidBindStatus } from '../../../../boundThreepids';
 
 /*
 TODO: Improve the UX for everything in here.
@@ -51,6 +50,11 @@ export class PhoneNumber extends React.Component {
         };
     }
 
+    componentWillReceiveProps(nextProps) {
+        const { bound } = nextProps.msisdn;
+        this.setState({ bound });
+    }
+
     async changeBinding({ bind, label, errorTitle }) {
         const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog");
         const { medium, address } = this.props.msisdn;
@@ -206,27 +210,14 @@ export class PhoneNumber extends React.Component {
 }
 
 export default class PhoneNumbers extends React.Component {
-    constructor() {
+    static propTypes = {
-        super();
+        msisdns: PropTypes.array.isRequired,
-
-        this.state = {
-            loaded: false,
-            msisdns: [],
-        };
-    }
-
-    async componentWillMount() {
-        const client = MatrixClientPeg.get();
-
-        const msisdns = await getThreepidBindStatus(client, 'msisdn');
-
-        this.setState({ msisdns });
     }
 
     render() {
         let content;
-        if (this.state.msisdns.length > 0) {
+        if (this.props.msisdns.length > 0) {
-            content = this.state.msisdns.map((e) => {
+            content = this.props.msisdns.map((e) => {
                 return <PhoneNumber msisdn={e} key={e.address} />;
             });
         } else {

src/components/views/settings/tabs/user/GeneralUserSettingsTab.js

@@ -37,6 +37,7 @@ import {Service, startTermsFlow} from "../../../../../Terms";
 import {SERVICE_TYPES} from "matrix-js-sdk";
 import IdentityAuthClient from "../../../../../IdentityAuthClient";
 import {abbreviateUrl} from "../../../../../utils/UrlUtils";
+import { getThreepidsWithBindStatus } from '../../../../../boundThreepids';
 
 export default class GeneralUserSettingsTab extends React.Component {
     static propTypes = {
@@ -58,17 +59,20 @@ export default class GeneralUserSettingsTab extends React.Component {
                 // agreedUrls,          // From the startTermsFlow callback
                 // resolve,             // Promise resolve function for startTermsFlow callback
             },
+            emails: [],
+            msisdns: [],
         };
 
         this.dispatcherRef = dis.register(this._onAction);
     }
 
     async componentWillMount() {
-        const serverRequiresIdServer = await MatrixClientPeg.get().doesServerRequireIdServerParam();
+        const cli = MatrixClientPeg.get();
+
+        const serverRequiresIdServer = await cli.doesServerRequireIdServerParam();
         this.setState({serverRequiresIdServer});
 
-        // Check to see if terms need accepting
+        this._getThreepidState();
-        this._checkTerms();
     }
 
     componentWillUnmount() {
@@ -78,10 +82,31 @@ export default class GeneralUserSettingsTab extends React.Component {
     _onAction = (payload) => {
         if (payload.action === 'id_server_changed') {
             this.setState({haveIdServer: Boolean(MatrixClientPeg.get().getIdentityServerUrl())});
-            this._checkTerms();
+            this._getThreepidState();
         }
     };
 
+    _onEmailsChange = (emails) => {
+        this.setState({ emails });
+    }
+
+    _onMsisdnsChange = (msisdns) => {
+        this.setState({ msisdns });
+    }
+
+    async _getThreepidState() {
+        const cli = MatrixClientPeg.get();
+
+        // Check to see if terms need accepting
+        this._checkTerms();
+
+        // Need to get 3PIDs generally for Account section and possibly also for
+        // Discovery (assuming we have an IS and terms are agreed).
+        const threepids = await getThreepidsWithBindStatus(cli);
+        this.setState({ emails: threepids.filter((a) => a.medium === 'email') });
+        this.setState({ msisdns: threepids.filter((a) => a.medium === 'msisdn') });
+    }
+
     async _checkTerms() {
         if (!this.state.haveIdServer) {
             this.setState({idServerHasUnsignedTerms: false});
@@ -91,7 +116,7 @@ export default class GeneralUserSettingsTab extends React.Component {
         // 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 authClient = new IdentityAuthClient();
-        const idAccessToken = await authClient.getAccessToken(/*check=*/false);
+        const idAccessToken = await authClient.getAccessToken({ check: false });
         startTermsFlow([new Service(
             SERVICE_TYPES.IS,
             MatrixClientPeg.get().getIdentityServerUrl(),
@@ -200,10 +225,16 @@ export default class GeneralUserSettingsTab extends React.Component {
         if (this.state.haveIdServer || this.state.serverRequiresIdServer === false) {
             threepidSection = <div>
                 <span className="mx_SettingsTab_subheading">{_t("Email addresses")}</span>
-                <EmailAddresses />
+                <EmailAddresses
+                    emails={this.state.emails}
+                    onEmailsChange={this._onEmailsChange}
+                />
 
                 <span className="mx_SettingsTab_subheading">{_t("Phone numbers")}</span>
-                <PhoneNumbers />
+                <PhoneNumbers
+                    msisdns={this.state.msisdns}
+                    onMsisdnsChange={this._onMsisdnsChange}
+                />
             </div>;
         } else if (this.state.serverRequiresIdServer === null) {
             threepidSection = <Spinner />;
@@ -279,10 +310,10 @@ export default class GeneralUserSettingsTab extends React.Component {
 
         const threepidSection = this.state.haveIdServer ? <div className='mx_GeneralUserSettingsTab_discovery'>
             <span className="mx_SettingsTab_subheading">{_t("Email addresses")}</span>
-            <EmailAddresses />
+            <EmailAddresses emails={this.state.emails} />
 
             <span className="mx_SettingsTab_subheading">{_t("Phone numbers")}</span>
-            <PhoneNumbers />
+            <PhoneNumbers msisdns={this.state.msisdns} />
         </div> : null;
 
         return (