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, name, permissions } = request; for (const permission of permissions) { if (!userSurfacedPermissionsSet.has(permission)) { @@ -135,16 +136,43 @@ const row = [id, community, name, permissionsBlob, time]; let query = SQL``; - if (action === 'create_role') { + if (request.action === 'create_role') { query = SQL` INSERT INTO roles (id, thread, name, permissions, creation_time) VALUES (${row}) `; - } else if (action === 'edit_role') { - throw new ServerError("unimplemented: can't edit roles yet"); - } - await dbQuery(query); + await dbQuery(query); + } else if (request.action === 'edit_role') { + const { existingRoleID } = request; + + 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) + 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, forcePermissionRecalculation: 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 @@ -351,12 +351,22 @@ ); } -const roleModificationRequestInputValidator = tShape({ - community: tID, - name: t.String, - permissions: t.list(userSurfacedPermissionValidator), - action: t.enums.of(['create_role', 'edit_role']), -}); +const roleModificationRequestInputValidator: TUnion = + t.union([ + tShape({ + community: tID, + name: t.String, + permissions: t.list(userSurfacedPermissionValidator), + action: t.enums.of(['create_role']), + }), + tShape({ + community: tID, + existingRoleID: tID, + name: t.String, + permissions: t.list(userSurfacedPermissionValidator), + action: t.enums.of(['edit_role']), + }), + ]); export const roleModificationResultValidator: TInterface = tShape({ 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 @@ -76,6 +76,7 @@ // -1 role means to set the user as a "ghost" (former member) type ChangeRoleOptions = { +setNewMembersToUnread?: boolean, + +forcePermissionRecalculation?: boolean, }; type ChangeRoleMemberInfo = { permissionsFromParent?: ?ThreadPermissionsBlob, @@ -90,6 +91,7 @@ const intent = role === -1 || role === 0 ? 'leave' : 'join'; const setNewMembersToUnread = options?.setNewMembersToUnread && intent === 'join'; + const forcePermissionRecalculation = options?.forcePermissionRecalculation; if (userIDs.length === 0) { return { @@ -197,8 +199,13 @@ const oldPermissions = existingMembership?.oldPermissions ?? null; const oldPermissionsForChildren = existingMembership?.oldPermissionsForChildren ?? null; + const shouldForceRecalculation = !!forcePermissionRecalculation; - if (existingMembership && oldRole === intendedRole) { + if ( + existingMembership && + oldRole === intendedRole && + !shouldForceRecalculation + ) { // If the old role is the same as the new one, we have nothing to update continue; } else if (Number(oldRole) > 0 && role === null) { 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,18 @@ import type { Viewer } from '../session/viewer.js'; import RelationshipChangeset from '../utils/relationship-changeset.js'; +type UpdateRoleOptions = { + +silenceNewMessages?: boolean, + +forcePermissionRecalculation?: boolean, +}; async function updateRole( viewer: Viewer, request: RoleChangeRequest, + options?: UpdateRoleOptions, ): Promise { + const silenceNewMessages = options?.silenceNewMessages; + const forcePermissionRecalculation = options?.forcePermissionRecalculation; + if (!viewer.loggedIn) { throw new ServerError('not_logged_in'); } @@ -136,8 +144,29 @@ throw new ServerError('invalid_parameters'); } - const changeset = await changeRole(request.threadID, memberIDs, request.role); - const { viewerUpdates } = await commitMembershipChangeset(viewer, changeset); + const changeset = await changeRole( + request.threadID, + memberIDs, + request.role, + { + forcePermissionRecalculation, + }, + ); + + const shouldForcePermissionRecalculation = !!forcePermissionRecalculation; + let commitMembershipChangesetOptions = {}; + + if (shouldForcePermissionRecalculation) { + commitMembershipChangesetOptions = { + changedThreadIDs: new Set([request.threadID]), + }; + } + + const { viewerUpdates } = await commitMembershipChangeset( + viewer, + changeset, + commitMembershipChangesetOptions, + ); const messageData = { type: messageTypes.CHANGE_ROLE, @@ -148,7 +177,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 @@ -397,13 +397,23 @@ +threadID: string, }; -export type RoleModificationRequest = { +type CreateRoleAction = { +community: string, +name: string, +permissions: $ReadOnlyArray, - +action: 'create_role' | 'edit_role', + +action: 'create_role', }; +type EditRoleAction = { + +community: string, + +existingRoleID: string, + +name: string, + +permissions: $ReadOnlyArray, + +action: 'edit_role', +}; + +export type RoleModificationRequest = CreateRoleAction | EditRoleAction; + export type RoleModificationResult = { +threadInfo: RawThreadInfo, +updatesResult: { 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); @@ -37,7 +38,7 @@ ); const onPressCreate = React.useCallback(() => { - if (threadRoleNames.includes(roleName)) { + if (threadRoleNames.includes(roleName) && action === 'create_role') { setRoleCreationFailed(true); return; } @@ -46,6 +47,7 @@ modifyCommunityRoleActionTypes, callModifyCommunityRole({ community: threadInfo.id, + existingRoleID, action, name: roleName, permissions: [...rolePermissions], @@ -58,6 +60,7 @@ dispatchActionPromise, threadInfo, action, + existingRoleID, roleName, rolePermissions, navigation, @@ -65,6 +68,7 @@ threadRoleNames, ]); + const headerRightText = action === 'create_role' ? 'Create' : 'Save'; const shouldHeaderRightBeDisabled = roleName.length === 0; const createButton = React.useMemo(() => { const textStyle = shouldHeaderRightBeDisabled @@ -76,11 +80,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 @@ -26,6 +26,7 @@ export type CreateRolesScreenParams = { +threadInfo: ThreadInfo, +action: 'create_role' | 'edit_role', + +existingRoleID?: string, +roleName: string, +rolePermissions: $ReadOnlySet, }; @@ -43,6 +44,7 @@ const { threadInfo, action, + existingRoleID, roleName: defaultRoleName, rolePermissions: defaultRolePermissions, } = props.route.params; @@ -123,10 +125,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 filteredUserSurfacedPermissionOptions = 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);