diff --git a/keyserver/src/creators/role-creator.js b/keyserver/src/creators/role-creator.js --- a/keyserver/src/creators/role-creator.js +++ b/keyserver/src/creators/role-creator.js @@ -27,6 +27,7 @@ } from '../fetchers/thread-fetchers.js'; import { checkThreadPermission } from '../fetchers/thread-permission-fetchers.js'; import type { Viewer } from '../session/viewer.js'; +import { updateRole } from '../updaters/thread-updaters.js'; type InitialRoles = { +default: RoleInfo, @@ -93,7 +94,7 @@ throw new ServerError('invalid_credentials'); } - const { community, name, permissions, action } = request; + const { community, existingRoleID, name, permissions, action } = request; for (const permission of permissions) { if (!userSurfacedPermissionsSet.has(permission)) { @@ -140,11 +141,40 @@ INSERT INTO roles (id, thread, name, permissions, creation_time) VALUES (${row}) `; + + await dbQuery(query); } else if (action === 'edit_role') { - throw new ServerError("unimplemented: can't edit roles yet"); - } + query = SQL` + UPDATE roles + SET name = ${name}, permissions = ${permissionsBlob} + WHERE id = ${existingRoleID} + `; - await dbQuery(query); + await dbQuery(query); + + // The `updateRole` needs to occur after the role has been updated + // in the database because it will want the most up to date role info + // (permissions / name) + if (!existingRoleID) { + throw new ServerError('invalid_parameters'); + } + + const membersWithRole = threadInfo.members + .filter(memberInfo => memberInfo.role === existingRoleID) + .map(memberInfo => memberInfo.id); + + if (membersWithRole.length > 0) { + await updateRole( + viewer, + { + threadID: community, + role: existingRoleID, + memberIDs: membersWithRole, + }, + { silenceNewMessages: true }, + ); + } + } const fetchServerThreadInfosResult = await fetchServerThreadInfos({ threadID: community, diff --git a/keyserver/src/responders/thread-responders.js b/keyserver/src/responders/thread-responders.js --- a/keyserver/src/responders/thread-responders.js +++ b/keyserver/src/responders/thread-responders.js @@ -353,6 +353,7 @@ const roleModificationRequestInputValidator = tShape({ community: tID, + existingRoleID: t.maybe(tID), name: t.String, permissions: t.list(userSurfacedPermissionValidator), action: t.enums.of(['create_role', 'edit_role']), diff --git a/keyserver/src/updaters/thread-permission-updaters.js b/keyserver/src/updaters/thread-permission-updaters.js --- a/keyserver/src/updaters/thread-permission-updaters.js +++ b/keyserver/src/updaters/thread-permission-updaters.js @@ -194,14 +194,10 @@ for (const userID of userIDs) { const existingMembership = existingMembershipInfo.get(userID); const oldRole = existingMembership?.oldRole ?? '-1'; - const oldPermissions = existingMembership?.oldPermissions ?? null; const oldPermissionsForChildren = existingMembership?.oldPermissionsForChildren ?? null; - if (existingMembership && oldRole === intendedRole) { - // If the old role is the same as the new one, we have nothing to update - continue; - } else if (Number(oldRole) > 0 && role === null) { + if (Number(oldRole) > 0 && role === null) { // In the case where we're just trying to add somebody to a thread, if // they already have a role with a nonzero role then we don't need to do // anything @@ -248,15 +244,6 @@ 'missing', ); } - if ( - existingMembership && - _isEqual(permissions)(oldPermissions) && - oldRole === newRole - ) { - // This thread and all of its descendants need no updates for this user, - // since the corresponding memberships row is unchanged by this operation - continue; - } if (permissions) { membershipRows.push({ diff --git a/keyserver/src/updaters/thread-updaters.js b/keyserver/src/updaters/thread-updaters.js --- a/keyserver/src/updaters/thread-updaters.js +++ b/keyserver/src/updaters/thread-updaters.js @@ -62,10 +62,16 @@ import type { Viewer } from '../session/viewer.js'; import RelationshipChangeset from '../utils/relationship-changeset.js'; +type UpdateRoleOptions = { + +silenceNewMessages?: boolean, +}; async function updateRole( viewer: Viewer, request: RoleChangeRequest, + options?: UpdateRoleOptions, ): Promise { + const silenceNewMessages = options?.silenceNewMessages; + if (!viewer.loggedIn) { throw new ServerError('not_logged_in'); } @@ -148,7 +154,10 @@ newRole: request.role, roleName: threadInfo.roles[request.role].name, }; - const newMessageInfos = await createMessages(viewer, [messageData]); + let newMessageInfos = []; + if (!silenceNewMessages) { + newMessageInfos = await createMessages(viewer, [messageData]); + } return { updatesResult: { newUpdates: viewerUpdates }, newMessageInfos }; } diff --git a/lib/types/thread-types.js b/lib/types/thread-types.js --- a/lib/types/thread-types.js +++ b/lib/types/thread-types.js @@ -399,6 +399,7 @@ export type RoleModificationRequest = { +community: string, + +existingRoleID: ?string, +name: string, +permissions: $ReadOnlyArray, +action: 'create_role' | 'edit_role', diff --git a/native/roles/create-roles-header-right-button.react.js b/native/roles/create-roles-header-right-button.react.js --- a/native/roles/create-roles-header-right-button.react.js +++ b/native/roles/create-roles-header-right-button.react.js @@ -23,7 +23,8 @@ }; function CreateRolesHeaderRightButton(props: Props): React.Node { - const { threadInfo, action, roleName, rolePermissions } = props.route.params; + const { threadInfo, action, existingRoleID, roleName, rolePermissions } = + props.route.params; const { setRoleCreationFailed } = props; const navigation = useNavigation(); const styles = useStyles(unboundStyles); @@ -33,7 +34,7 @@ const onPressCreate = React.useCallback(() => { const threadRoleNames = values(threadInfo.roles).map(role => role.name); - if (threadRoleNames.includes(roleName)) { + if (threadRoleNames.includes(roleName) && action === 'create_role') { setRoleCreationFailed(true); return; } @@ -42,6 +43,7 @@ modifyCommunityRoleActionTypes, callModifyCommunityRole({ community: threadInfo.id, + existingRoleID, action, name: roleName, permissions: rolePermissions, @@ -54,12 +56,14 @@ dispatchActionPromise, threadInfo, action, + existingRoleID, roleName, rolePermissions, navigation, setRoleCreationFailed, ]); + const headerRightText = action === 'create_role' ? 'Create' : 'Save'; const shouldHeaderRightBeDisabled = roleName.length === 0; const createButton = React.useMemo(() => { const textStyle = shouldHeaderRightBeDisabled @@ -71,11 +75,12 @@ onPress={onPressCreate} disabled={shouldHeaderRightBeDisabled} > - Create + {headerRightText} ); }, [ shouldHeaderRightBeDisabled, + headerRightText, styles.createButtonDisabled, styles.createButton, onPressCreate, diff --git a/native/roles/create-roles-screen.react.js b/native/roles/create-roles-screen.react.js --- a/native/roles/create-roles-screen.react.js +++ b/native/roles/create-roles-screen.react.js @@ -28,6 +28,7 @@ export type CreateRolesScreenParams = { +threadInfo: ThreadInfo, +action: 'create_role' | 'edit_role', + +existingRoleID?: string, +roleName: string, +rolePermissions: $ReadOnlyArray, }; @@ -45,6 +46,7 @@ const { threadInfo, action, + existingRoleID, roleName: defaultRoleName, rolePermissions: defaultRolePermissions, } = props.route.params; @@ -122,10 +124,18 @@ props.navigation.setParams({ threadInfo, action, + existingRoleID, roleName: customRoleName, rolePermissions: selectedPermissions, }), - [props.navigation, threadInfo, action, customRoleName, selectedPermissions], + [ + props.navigation, + threadInfo, + action, + existingRoleID, + customRoleName, + selectedPermissions, + ], ); const modifiedUserSurfacedPermissionOptions = React.useMemo( diff --git a/native/roles/role-panel-entry.react.js b/native/roles/role-panel-entry.react.js --- a/native/roles/role-panel-entry.react.js +++ b/native/roles/role-panel-entry.react.js @@ -1,6 +1,7 @@ // @flow import { useActionSheet } from '@expo/react-native-action-sheet'; +import invariant from 'invariant'; import * as React from 'react'; import { View, Text, TouchableOpacity, Platform } from 'react-native'; import { useSafeAreaInsets } from 'react-native-safe-area-context'; @@ -28,6 +29,15 @@ props; const styles = useStyles(unboundStyles); + const existingRoleID = React.useMemo( + () => + Object.keys(threadInfo.roles).find( + roleID => threadInfo.roles[roleID].name === roleName, + ), + [roleName, threadInfo.roles], + ); + invariant(existingRoleID, 'Role ID must exist for an existing role'); + const options = React.useMemo(() => { const availableOptions = ['Edit role']; @@ -50,12 +60,20 @@ navigation.navigate(CreateRolesScreenRouteName, { threadInfo, action: 'edit_role', + existingRoleID, roleName, rolePermissions, }); } }, - [navigation, options, roleName, rolePermissions, threadInfo], + [ + navigation, + options, + existingRoleID, + roleName, + rolePermissions, + threadInfo, + ], ); const activeTheme = useSelector(state => state.globalThemeInfo.activeTheme);