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

Only show editing UI for aliases/related_groups for users /w power #1529

Merged
merged 3 commits into from
Oct 25, 2017
Merged

Only show editing UI for aliases/related_groups for users /w power #1529

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee7ebc4
Only show editing UI for aliases/related_groups for users /w power
Oct 24, 2017
640d88f
Typo
Oct 24, 2017
e9ce7f9
canSetRelatedRooms -> canSetRelatedGroups
Oct 24, 2017
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
22 changes: 13 additions & 9 deletions src/components/views/elements/EditableItemList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ module.exports = React.createClass({
onNewItemChanged: PropTypes.func,
onItemAdded: PropTypes.func,
onItemEdited: PropTypes.func,
onItemRemoved: PropTypes. func,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea how this is even possible but empirically it's not a syntax error. Babel might be dealing with it?

Copy link
Member

Choose a reason for hiding this comment

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

heh, I can well believe that the dot operator accepts whitespace around it

onItemRemoved: PropTypes.func,

canEdit: PropTypes.bool,
},

getDefaultProps: function() {
Expand Down Expand Up @@ -136,14 +138,16 @@ module.exports = React.createClass({
{ label }
</div>
{ editableItems }
<EditableItem
key={-1}
initialValue={this.props.newItem}
onAdd={this.onItemAdded}
onChange={this.onNewItemChanged}
addOnChange={true}
placeholder={this.props.placeholder}
/>
{ this.props.canEdit ?
<EditableItem
key={-1}
initialValue={this.props.newItem}
onAdd={this.onItemAdded}
onChange={this.onNewItemChanged}
addOnChange={true}
placeholder={this.props.placeholder}
/> : <div />
}
</div>);
},
});
1 change: 1 addition & 0 deletions src/components/views/room_settings/AliasSettings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ module.exports = React.createClass({
items={this.state.domainToAliases[localDomain] || []}
newItem={this.state.newAlias}
onNewItemChanged={this.onNewAliasChanged}
canEdit={this.props.canSetAliases}
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it ends up preventing you from adding aliases local to your homeserver. The flag given to the class is for canonical aliases, which should certainly be restricted. However arbitrary aliases are different and bypass power levels: matrix-org/synapse#2402

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not convinced this is relevant here. canSetAliases is always true apparently, so this is more for clarity. canSetCanonicalAlias is for canonical aliases.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the current situation is that we hardcode canSetAliases to true but then also never actually read it anywhere in AliasSettings, right? We may as well either wire the thing up if it looks trivial enough to do so (which it looks like it is) or remove it.

onItemAdded={this.onLocalAliasAdded}
onItemEdited={this.onLocalAliasChanged}
onItemRemoved={this.onLocalAliasDeleted}
Expand Down
1 change: 1 addition & 0 deletions src/components/views/room_settings/RelatedGroupSettings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ module.exports = React.createClass({
items={this.state.newGroupsList}
className={"mx_RelatedGroupSettings"}
newItem={this.state.newGroupId}
canEdit={this.props.canSetRelatedRooms}
Copy link
Member

Choose a reason for hiding this comment

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

I probably missed this in an earlier PR. but should this be canSetRelatedGroups?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep!

onNewItemChanged={this.onNewGroupChanged}
onItemAdded={this.onGroupAdded}
onItemEdited={this.onGroupEdited}
Expand Down
1 change: 0 additions & 1 deletion src/components/views/rooms/RoomSettings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import Promise from 'bluebird';
import React from 'react';
import { _t, _tJsx, _td } from '../../../languageHandler';
import MatrixClientPeg from '../../../MatrixClientPeg';
import SdkConfig from '../../../SdkConfig';
import sdk from '../../../index';
import Modal from '../../../Modal';
import ObjectUtils from '../../../ObjectUtils';
Expand Down