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)) { @@ -141,11 +142,37 @@ VALUES (${row}) `; } 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); + // 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 (action === 'edit_role') { + if (!existingRoleID) { + throw new ServerError('invalid_parameters'); + } + + const membersWithRole = threadInfo.members.filter( + memberInfo => memberInfo.role === existingRoleID, + ); + await updateRole( + viewer, + { + threadID: community, + role: existingRoleID, + memberIDs: membersWithRole.map(memberInfo => memberInfo.id), + }, + { 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 @@ -428,6 +428,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 navigation = useNavigation(); const styles = useStyles(unboundStyles); @@ -32,7 +33,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') { displayActionResultModal( 'There is already a role with this name in the community', ); @@ -43,6 +44,7 @@ modifyCommunityRoleActionTypes, callModifyCommunityRole({ community: threadInfo.id, + existingRoleID, action, name: roleName, permissions: rolePermissions, @@ -55,11 +57,13 @@ dispatchActionPromise, threadInfo, action, + existingRoleID, roleName, rolePermissions, navigation, ]); + 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; @@ -111,10 +113,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);