Merge pull request #4187 from matrix-org/bwindels/altaliases

Allow editing of alt_aliases according to MSC2432

res/css/views/room_settings/_AliasSettings.scss

@@ -26,3 +26,7 @@ limitations under the License.
     outline: none;
     box-shadow: none;
 }
+
+.mx_AliasSettings summary {
+    cursor: pointer;
+}

src/components/views/elements/EditableItemList.js

@@ -123,8 +123,9 @@ export default class EditableItemList extends React.Component {
             <form onSubmit={this._onItemAdded} autoComplete="off"
                   noValidate={true} className="mx_EditableItemList_newItem">
                 <Field id={`mx_EditableItemList_new_${this.props.id}`} label={this.props.placeholder} type="text"
-                       autoComplete="off" value={this.props.newItem || ""} onChange={this._onNewItemChanged} />
+                       autoComplete="off" value={this.props.newItem || ""} onChange={this._onNewItemChanged}
-                <AccessibleButton onClick={this._onItemAdded} kind="primary">
+                       list={this.props.suggestionsListId} />
+                <AccessibleButton onClick={this._onItemAdded} kind="primary" type="submit">
                     {_t("Add")}
                 </AccessibleButton>
             </form>

src/components/views/elements/Field.js

@@ -32,6 +32,8 @@ export default class Field extends React.PureComponent {
         element: PropTypes.oneOf(["input", "select", "textarea"]),
         // The field's type (when used as an <input>). Defaults to "text".
         type: PropTypes.string,
+        // id of a <datalist> element for suggestions
+        list: PropTypes.string,
         // The field's label string.
         label: PropTypes.string,
         // The field's placeholder string. Defaults to the label.
@@ -157,7 +159,7 @@ export default class Field extends React.PureComponent {
     render() {
         const {
             element, prefix, postfix, className, onValidate, children,
-            tooltipContent, flagInvalid, tooltipClassName, ...inputProps} = this.props;
+            tooltipContent, flagInvalid, tooltipClassName, list, ...inputProps} = this.props;
 
         const inputElement = element || "input";
 
@@ -169,6 +171,7 @@ export default class Field extends React.PureComponent {
         inputProps.onFocus = this.onFocus;
         inputProps.onChange = this.onChange;
         inputProps.onBlur = this.onBlur;
+        inputProps.list = list;
 
         const fieldInput = React.createElement(inputElement, inputProps, children);
 

src/components/views/room_settings/AliasSettings.js

@@ -46,6 +46,11 @@ class EditableAliasesList extends EditableItemList {
     };
 
     _renderNewItemField() {
+        // if we don't need the RoomAliasField,
+        // we don't need to overriden version of _renderNewItemField
+        if (!this.props.domain) {
+            return super._renderNewItemField();
+        }
         const RoomAliasField = sdk.getComponent('views.elements.RoomAliasField');
         const onChange = (alias) => this._onNewItemChanged({target: {value: alias}});
         return (
@@ -87,91 +92,63 @@ export default class AliasSettings extends React.Component {
         super(props);
 
         const state = {
-            domainToAliases: {}, // { domain.com => [#alias1:domain.com, #alias2:domain.com] }
+            altAliases: [], // [ #alias:domain.tld, ... ]
-            remoteDomains: [], // [ domain.com, foobar.com ]
+            localAliases: [], // [ #alias:my-hs.tld, ... ]
-            canonicalAlias: null, // #canonical:domain.com
+            canonicalAlias: null, // #canonical:domain.tld
             updatingCanonicalAlias: false,
-            localAliasesLoading: true,
+            localAliasesLoading: false,
         };
 
         if (props.canonicalAliasEvent) {
-            state.canonicalAlias = props.canonicalAliasEvent.getContent().alias;
+            const content = props.canonicalAliasEvent.getContent();
+            const altAliases = content.alt_aliases;
+            if (Array.isArray(altAliases)) {
+                state.altAliases = altAliases.slice();
+            }
+            state.canonicalAlias = content.alias;
         }
 
         this.state = state;
     }
 
-    async componentWillMount() {
+    componentDidMount() {
-        const cli = MatrixClientPeg.get();
+        if (this.props.canSetCanonicalAlias) {
+            // load local aliases for providing recommendations
+            // for the canonical alias and alt_aliases
+            this.loadLocalAliases();
+        }
+    }
+
+    async loadLocalAliases() {
+        this.setState({ localAliasesLoading: true });
         try {
+            const cli = MatrixClientPeg.get();
+            let localAliases = [];
             if (await cli.doesServerSupportUnstableFeature("org.matrix.msc2432")) {
                 const response = await cli.unstableGetLocalAliases(this.props.roomId);
-                const localAliases = response.aliases;
+                if (Array.isArray(response.aliases)) {
-                const localDomain = cli.getDomain();
+                    localAliases = response.aliases;
-                const domainToAliases = Object.assign(
-                    {},
-                    // FIXME, any localhost alt_aliases will be ignored as they are overwritten by localAliases
-                    this.aliasesToDictionary(this._getAltAliases()),
-                    {[localDomain]: localAliases || []},
-                );
-                const remoteDomains = Object.keys(domainToAliases).filter((domain) => {
-                    return domain !== localDomain && domainToAliases[domain].length > 0;
-                });
-                this.setState({ domainToAliases, remoteDomains });
-            } else {
-                const state = {};
-                const localDomain = cli.getDomain();
-                state.domainToAliases = this.aliasEventsToDictionary(this.props.aliasEvents || []);
-                state.remoteDomains = Object.keys(state.domainToAliases).filter((domain) => {
-                    return domain !== localDomain && state.domainToAliases[domain].length > 0;
-                });
-                this.setState(state);
                 }
+            }
+            this.setState({ localAliases });
         } finally {
-            this.setState({localAliasesLoading: false});
+            this.setState({ localAliasesLoading: false });
         }
     }
 
-    aliasesToDictionary(aliases) {
-        return aliases.reduce((dict, alias) => {
-            const domain = alias.split(":")[1];
-            dict[domain] = dict[domain] || [];
-            dict[domain].push(alias);
-            return dict;
-        }, {});
-    }
-
-    aliasEventsToDictionary(aliasEvents) { // m.room.alias events
-        const dict = {};
-        aliasEvents.forEach((event) => {
-            dict[event.getStateKey()] = (
-                (event.getContent().aliases || []).slice() // shallow-copy
-            );
-        });
-        return dict;
-    }
-
-    _getAltAliases() {
-        if (this.props.canonicalAliasEvent) {
-            const altAliases = this.props.canonicalAliasEvent.getContent().alt_aliases;
-            if (Array.isArray(altAliases)) {
-                return altAliases;
-            }
-        }
-        return [];
-    }
-
     changeCanonicalAlias(alias) {
         if (!this.props.canSetCanonicalAlias) return;
 
+        const oldAlias = this.state.canonicalAlias;
         this.setState({
             canonicalAlias: alias,
             updatingCanonicalAlias: true,
         });
 
-        const eventContent = {};
+        const eventContent = {
-        const altAliases = this._getAltAliases();
+            alt_aliases: this.state.altAliases,
-        if (altAliases) eventContent["alt_aliases"] = altAliases;
+        };
+
         if (alias) eventContent["alias"] = alias;
 
         MatrixClientPeg.get().sendStateEvent(this.props.roomId, "m.room.canonical_alias",
@@ -184,6 +161,39 @@ export default class AliasSettings extends React.Component {
                     "or a temporary failure occurred.",
                 ),
             });
+            this.setState({canonicalAlias: oldAlias});
+        }).finally(() => {
+            this.setState({updatingCanonicalAlias: false});
+        });
+    }
+
+    changeAltAliases(altAliases) {
+        if (!this.props.canSetCanonicalAlias) return;
+
+        this.setState({
+            updatingCanonicalAlias: true,
+            altAliases,
+        });
+
+        const eventContent = {};
+
+        if (this.state.canonicalAlias) {
+            eventContent.alias = this.state.canonicalAlias;
+        }
+        if (altAliases) {
+            eventContent["alt_aliases"] = altAliases;
+        }
+
+        MatrixClientPeg.get().sendStateEvent(this.props.roomId, "m.room.canonical_alias",
+            eventContent, "").catch((err) => {
+            console.error(err);
+            Modal.createTrackedDialog('Error updating alternative addresses', '', ErrorDialog, {
+                title: _t("Error updating main address"),
+                description: _t(
+                    "There was an error updating the room's alternative addresses. " +
+                    "It may not be allowed by the server or a temporary failure occurred.",
+                ),
+            });
         }).finally(() => {
             this.setState({updatingCanonicalAlias: false});
         });
@@ -200,16 +210,10 @@ export default class AliasSettings extends React.Component {
         if (!alias.includes(':')) alias += ':' + localDomain;
 
         MatrixClientPeg.get().createAlias(alias, this.props.roomId).then(() => {
-            const localAliases = this.state.domainToAliases[localDomain] || [];
-            const domainAliases = Object.assign({}, this.state.domainToAliases);
-            domainAliases[localDomain] = [...localAliases, alias];
-
             this.setState({
-                domainToAliases: domainAliases,
+                localAliases: this.state.localAliases.concat(alias),
-                // Reset the add field
+                newAlias: null,
-                newAlias: "",
             });
-
             if (!this.state.canonicalAlias) {
                 this.changeCanonicalAlias(alias);
             }
@@ -226,18 +230,13 @@ export default class AliasSettings extends React.Component {
     };
 
     onLocalAliasDeleted = (index) => {
-        const localDomain = MatrixClientPeg.get().getDomain();
+        const alias = this.state.localAliases[index];
-
-        const alias = this.state.domainToAliases[localDomain][index];
-
         // TODO: In future, we should probably be making sure that the alias actually belongs
         // to this room. See https://github.com/vector-im/riot-web/issues/7353
         MatrixClientPeg.get().deleteAlias(alias).then(() => {
-            const localAliases = this.state.domainToAliases[localDomain].filter((a) => a !== alias);
+            const localAliases = this.state.localAliases.slice();
-            const domainAliases = Object.assign({}, this.state.domainToAliases);
+            localAliases.splice(index);
-            domainAliases[localDomain] = localAliases;
+            this.setState({localAliases});
-
-            this.setState({domainToAliases: domainAliases});
 
             if (this.state.canonicalAlias === alias) {
                 this.changeCanonicalAlias(null);
@@ -254,10 +253,48 @@ export default class AliasSettings extends React.Component {
         });
     };
 
+    onLocalAliasesToggled = (event) => {
+        // expanded
+        if (event.target.open) {
+            // if local aliases haven't been preloaded yet at component mount
+            if (!this.props.canSetCanonicalAlias && this.state.localAliases.length === 0) {
+                this.loadLocalAliases();
+            }
+        }
+    };
+
     onCanonicalAliasChange = (event) => {
         this.changeCanonicalAlias(event.target.value);
     };
 
+    onNewAltAliasChanged = (value) => {
+        this.setState({newAltAlias: value});
+    }
+
+    onAltAliasAdded = (alias) => {
+        const altAliases = this.state.altAliases.slice();
+        if (!altAliases.some(a => a.trim() === alias.trim())) {
+            altAliases.push(alias.trim());
+            this.changeAltAliases(altAliases);
+            this.setState({newAltAlias: ""});
+        }
+    }
+
+    onAltAliasDeleted = (index) => {
+        const altAliases = this.state.altAliases.slice();
+        altAliases.splice(index, 1);
+        this.changeAltAliases(altAliases);
+    }
+
+    _getAliases() {
+        return this.state.altAliases.concat(this._getLocalNonAltAliases());
+    }
+
+    _getLocalNonAltAliases() {
+        const {altAliases} = this.state;
+        return this.state.localAliases.filter(alias => !altAliases.includes(alias));
+    }
+
     render() {
         const localDomain = MatrixClientPeg.get().getDomain();
 
@@ -269,15 +306,13 @@ export default class AliasSettings extends React.Component {
                    element='select' id='canonicalAlias' label={_t('Main address')}>
                 <option value="" key="unset">{ _t('not specified') }</option>
                 {
-                    Object.keys(this.state.domainToAliases).map((domain, i) => {
+                    this._getAliases().map((alias, i) => {
-                        return this.state.domainToAliases[domain].map((alias, j) => {
                         if (alias === this.state.canonicalAlias) found = true;
                         return (
-                                <option value={alias} key={i + "_" + j}>
+                            <option value={alias} key={i}>
                                 { alias }
                             </option>
                         );
-                        });
                     })
                 }
                 {
@@ -289,53 +324,58 @@ export default class AliasSettings extends React.Component {
             </Field>
         );
 
-        let remoteAliasesSection;
-        if (this.state.remoteDomains.length) {
-            remoteAliasesSection = (
-                <div>
-                    <div>
-                        { _t("Remote addresses for this room:") }
-                    </div>
-                    <ul>
-                        { this.state.remoteDomains.map((domain, i) => {
-                            return this.state.domainToAliases[domain].map((alias, j) => {
-                                return <li key={i + "_" + j}>{alias}</li>;
-                            });
-                        }) }
-                    </ul>
-                </div>
-            );
-        }
-
         let localAliasesList;
         if (this.state.localAliasesLoading) {
             const Spinner = sdk.getComponent("elements.Spinner");
             localAliasesList = <Spinner />;
         } else {
-            localAliasesList = <EditableAliasesList
+            localAliasesList = (<EditableAliasesList
                 id="roomAliases"
                 className={"mx_RoomSettings_localAliases"}
-                items={this.state.domainToAliases[localDomain] || []}
+                items={this.state.localAliases}
                 newItem={this.state.newAlias}
                 onNewItemChanged={this.onNewAliasChanged}
                 canRemove={this.props.canSetAliases}
                 canEdit={this.props.canSetAliases}
                 onItemAdded={this.onLocalAliasAdded}
                 onItemRemoved={this.onLocalAliasDeleted}
-                itemsLabel={_t('Local addresses for this room:')}
                 noItemsLabel={_t('This room has no local addresses')}
                 placeholder={_t(
                     'New address (e.g. #foo:%(localDomain)s)', {localDomain: localDomain},
                 )}
                 domain={localDomain}
-            />;
+            />);
         }
 
         return (
             <div className='mx_AliasSettings'>
                 {canonicalAliasSection}
+                <datalist id="mx_AliasSettings_altRecommendations">
+                    {this._getLocalNonAltAliases().map(alias => {
+                        return <option value={alias} key={alias} />;
+                    })};
+                </datalist>
+                <EditableAliasesList
+                    id="roomAltAliases"
+                    className={"mx_RoomSettings_altAliases"}
+                    items={this.state.altAliases}
+                    newItem={this.state.newAltAlias}
+                    onNewItemChanged={this.onNewAltAliasChanged}
+                    canRemove={this.props.canSetCanonicalAlias}
+                    canEdit={this.props.canSetCanonicalAlias}
+                    onItemAdded={this.onAltAliasAdded}
+                    onItemRemoved={this.onAltAliasDeleted}
+                    suggestionsListId="mx_AliasSettings_altRecommendations"
+                    itemsLabel={_t('Alternative addresses for this room:')}
+                    noItemsLabel={_t('This room has no alternative addresses')}
+                    placeholder={_t(
+                        'New address (e.g. #foo:domain)',
+                    )}
+                />
+                <details onToggle={this.onLocalAliasesToggled}>
+                    <summary>{_t('Local addresses (unmoderated content)')}</summary>
                     {localAliasesList}
-                {remoteAliasesSection}
+                </details>
             </div>
         );
     }

src/i18n/strings/en_EN.json

@@ -1141,16 +1141,19 @@
     "Mark all as read": "Mark all as read",
     "Error updating main address": "Error updating main address",
     "There was an error updating the room's main address. It may not be allowed by the server or a temporary failure occurred.": "There was an error updating the room's main address. It may not be allowed by the server or a temporary failure occurred.",
+    "There was an error updating the room's alternative addresses. It may not be allowed by the server or a temporary failure occurred.": "There was an error updating the room's alternative addresses. It may not be allowed by the server or a temporary failure occurred.",
     "Error creating alias": "Error creating alias",
     "There was an error creating that alias. It may not be allowed by the server or a temporary failure occurred.": "There was an error creating that alias. It may not be allowed by the server or a temporary failure occurred.",
     "Error removing alias": "Error removing alias",
     "There was an error removing that alias. It may no longer exist or a temporary error occurred.": "There was an error removing that alias. It may no longer exist or a temporary error occurred.",
     "Main address": "Main address",
     "not specified": "not specified",
-    "Remote addresses for this room:": "Remote addresses for this room:",
-    "Local addresses for this room:": "Local addresses for this room:",
     "This room has no local addresses": "This room has no local addresses",
     "New address (e.g. #foo:%(localDomain)s)": "New address (e.g. #foo:%(localDomain)s)",
+    "Alternative addresses for this room:": "Alternative addresses for this room:",
+    "This room has no alternative addresses": "This room has no alternative addresses",
+    "New address (e.g. #foo:domain)": "New address (e.g. #foo:domain)",
+    "Local addresses (unmoderated content)": "Local addresses (unmoderated content)",
     "Error updating flair": "Error updating flair",
     "There was an error updating the flair for this room. The server may not allow it or a temporary error occurred.": "There was an error updating the flair for this room. The server may not allow it or a temporary error occurred.",
     "Invalid community ID": "Invalid community ID",

test/end-to-end-tests/src/usecases/room-settings.js

@@ -52,9 +52,11 @@ module.exports = async function changeRoomSettings(session, settings) {
 
     if (settings.alias) {
         session.log.step(`sets alias to ${settings.alias}`);
-        const aliasField = await session.query(".mx_RoomSettingsDialog .mx_AliasSettings input[type=text]");
+        const summary = await session.query(".mx_RoomSettingsDialog .mx_AliasSettings summary");
+        await summary.click();
+        const aliasField = await session.query(".mx_RoomSettingsDialog .mx_AliasSettings details input[type=text]");
         await session.replaceInputText(aliasField, settings.alias.substring(1, settings.alias.lastIndexOf(":")));
-        const addButton = await session.query(".mx_RoomSettingsDialog .mx_AliasSettings .mx_AccessibleButton");
+        const addButton = await session.query(".mx_RoomSettingsDialog .mx_AliasSettings details .mx_AccessibleButton");
         await addButton.click();
         await session.delay(10); // delay to give time for the validator to run and check the alias
         session.log.done();