From bf90932f4084113fe5c753c9428cbdfecdb31808 Mon Sep 17 00:00:00 2001 From: roo hutton Date: Tue, 25 Jun 2024 13:02:30 +0100 Subject: [PATCH] Merge pull request #18936 from overleaf/rh-viewer-no-comment [web] Remove access to commenting/chat for collaborators with Viewer permission GitOrigin-RevId: 03957cb8c04866318a4b94bdb72843e7d7a5a003 --- .../js/features/chat/components/chat-pane.tsx | 11 +++++++--- .../js/features/chat/context/chat-context.tsx | 6 ++++-- .../editor-navigation-toolbar-root.tsx | 5 ++++- .../components/toolbar-header.jsx | 4 +++- .../ide-react/context/permissions-context.tsx | 21 +++++++++++++++---- .../hooks/use-review-panel-state.ts | 14 +++++++++---- .../components/codemirror-editor.tsx | 6 +++++- .../js/shared/hooks/use-viewer-permissions.ts | 12 +++++++++++ .../components/toolbar-header.test.jsx | 5 +++-- 9 files changed, 66 insertions(+), 18 deletions(-) create mode 100644 services/web/frontend/js/shared/hooks/use-viewer-permissions.ts diff --git a/services/web/frontend/js/features/chat/components/chat-pane.tsx b/services/web/frontend/js/features/chat/components/chat-pane.tsx index ff62f7e5ed..6f812b1c77 100644 --- a/services/web/frontend/js/features/chat/components/chat-pane.tsx +++ b/services/web/frontend/js/features/chat/components/chat-pane.tsx @@ -11,13 +11,14 @@ import withErrorBoundary from '../../../infrastructure/error-boundary' import { FetchError } from '../../../infrastructure/fetch-json' import { useChatContext } from '../context/chat-context' import LoadingSpinner from '../../../shared/components/loading-spinner' +import useViewerPermissions from '@/shared/hooks/use-viewer-permissions' const MessageList = lazy(() => import('./message-list')) const ChatPane = React.memo(function ChatPane() { const { t } = useTranslation() - const { chatIsOpen } = useLayoutContext() + const { setChatIsOpen, chatIsOpen } = useLayoutContext() const user = useUserContext() const { @@ -46,13 +47,17 @@ const ChatPane = React.memo(function ChatPane() { 0 ) + const shouldShowChat = !useViewerPermissions() + // Keep the chat pane in the DOM to avoid resetting the form input and re-rendering MathJax content. const [chatOpenedOnce, setChatOpenedOnce] = useState(chatIsOpen) useEffect(() => { - if (chatIsOpen) { + if (chatIsOpen && shouldShowChat) { setChatOpenedOnce(true) + } else if (chatIsOpen && !shouldShowChat) { + setChatIsOpen(false) } - }, [chatIsOpen]) + }, [chatIsOpen, setChatIsOpen, shouldShowChat]) if (error) { // let user try recover from fetch errors diff --git a/services/web/frontend/js/features/chat/context/chat-context.tsx b/services/web/frontend/js/features/chat/context/chat-context.tsx index 65e6093c7c..a87eab6f3e 100644 --- a/services/web/frontend/js/features/chat/context/chat-context.tsx +++ b/services/web/frontend/js/features/chat/context/chat-context.tsx @@ -17,6 +17,7 @@ import { appendMessage, prependMessages } from '../utils/message-list-appender' import useBrowserWindow from '../../../shared/hooks/use-browser-window' import { useLayoutContext } from '../../../shared/context/layout-context' import { useIdeContext } from '@/shared/context/ide-context' +import useViewerPermissions from '@/shared/hooks/use-viewer-permissions' const PAGE_SIZE = 50 @@ -286,9 +287,10 @@ export const ChatProvider: FC = ({ children }) => { }, []) // Handling receiving messages over the socket + const hasViewerPermissions = useViewerPermissions() const { socket } = useIdeContext() useEffect(() => { - if (!socket) return + if (!socket || hasViewerPermissions) return function receivedMessage(message: any) { // If the message is from the current client id, then we are receiving the sent message back from the socket. @@ -304,7 +306,7 @@ export const ChatProvider: FC = ({ children }) => { socket.removeListener('new-chat-message', receivedMessage) } - }, [socket]) + }, [socket, hasViewerPermissions]) // Handle unread messages useEffect(() => { diff --git a/services/web/frontend/js/features/editor-navigation-toolbar/components/editor-navigation-toolbar-root.tsx b/services/web/frontend/js/features/editor-navigation-toolbar/components/editor-navigation-toolbar-root.tsx index 3a3477c859..215c737f06 100644 --- a/services/web/frontend/js/features/editor-navigation-toolbar/components/editor-navigation-toolbar-root.tsx +++ b/services/web/frontend/js/features/editor-navigation-toolbar/components/editor-navigation-toolbar-root.tsx @@ -6,6 +6,7 @@ import { useLayoutContext } from '../../../shared/context/layout-context' import { useProjectContext } from '../../../shared/context/project-context' import * as eventTracking from '../../../infrastructure/event-tracking' import { Doc } from '../../../../../types/doc' +import useViewerPermissions from '@/shared/hooks/use-viewer-permissions' function isOpentoString(open: boolean) { return open ? 'open' : 'close' @@ -44,6 +45,7 @@ const EditorNavigationToolbarRoot = React.memo( } = useLayoutContext() const { markMessagesAsRead, unreadMessageCount } = useChatContext() + const canViewChatAndTrackChanges = !useViewerPermissions() const toggleChatOpen = useCallback(() => { if (!chatIsOpen) { @@ -119,11 +121,12 @@ const EditorNavigationToolbarRoot = React.memo( hasPublishPermissions={ permissionsLevel === 'owner' || permissionsLevel === 'readAndWrite' } + chatVisible={!(isRestrictedTokenMember || !canViewChatAndTrackChanges)} projectName={projectName} renameProject={renameProject} hasRenamePermissions={permissionsLevel === 'owner'} openShareModal={openShareModal} - trackChangesVisible={trackChangesVisible} + trackChangesVisible={canViewChatAndTrackChanges && trackChangesVisible} /> ) } diff --git a/services/web/frontend/js/features/editor-navigation-toolbar/components/toolbar-header.jsx b/services/web/frontend/js/features/editor-navigation-toolbar/components/toolbar-header.jsx index 19478c6ba9..64d6d889f6 100644 --- a/services/web/frontend/js/features/editor-navigation-toolbar/components/toolbar-header.jsx +++ b/services/web/frontend/js/features/editor-navigation-toolbar/components/toolbar-header.jsx @@ -33,6 +33,7 @@ const ToolbarHeader = React.memo(function ToolbarHeader({ goToUser, isRestrictedTokenMember, hasPublishPermissions, + chatVisible, projectName, renameProject, hasRenamePermissions, @@ -89,7 +90,7 @@ const ToolbarHeader = React.memo(function ToolbarHeader({ - {!isRestrictedTokenMember && ( + {chatVisible && ( ( undefined @@ -40,6 +41,12 @@ const anonymousPermissionsMap: typeof permissionsMap = { owner: { ...permissionsMap.owner, comment: false }, } +const linkSharingWarningPermissionsMap: typeof permissionsMap = { + readOnly: { ...permissionsMap.readOnly, comment: false }, + readAndWrite: permissionsMap.readAndWrite, + owner: permissionsMap.owner, +} + export const PermissionsProvider: React.FC = ({ children }) => { const [permissions, setPermissions] = useScopeValue>('permissions') @@ -47,14 +54,20 @@ export const PermissionsProvider: React.FC = ({ children }) => { const { permissionsLevel } = useEditorContext() as { permissionsLevel: PermissionsLevel } + const hasViewerPermissions = useViewerPermissions() const anonymous = getMeta('ol-anonymous') useEffect(() => { - const activePermissionsMap = anonymous - ? anonymousPermissionsMap - : permissionsMap + let activePermissionsMap + if (hasViewerPermissions) { + activePermissionsMap = linkSharingWarningPermissionsMap + } else { + activePermissionsMap = anonymous + ? anonymousPermissionsMap + : permissionsMap + } setPermissions(activePermissionsMap[permissionsLevel]) - }, [anonymous, permissionsLevel, setPermissions]) + }, [anonymous, permissionsLevel, setPermissions, hasViewerPermissions]) useEffect(() => { if (connectionState.forceDisconnected) { diff --git a/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts b/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts index d16c72abf1..1bf99d62ce 100644 --- a/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts +++ b/services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts @@ -24,7 +24,6 @@ import { useEditorManagerContext, } from '@/features/ide-react/context/editor-manager-context' import { debugConsole } from '@/utils/debugging' -import { useEditorContext } from '@/shared/context/editor-context' import { deleteJSON, getJSON, postJSON } from '@/infrastructure/fetch-json' import ColorManager from '@/ide/colors/ColorManager' import RangesTracker from '@overleaf/ranges-tracker' @@ -65,7 +64,9 @@ import { EditOperation, } from '../../../../../../../types/change' import { RangesTrackerWithResolvedThreadIds } from '@/features/ide-react/editor/document-container' +import useViewerPermissions from '@/shared/hooks/use-viewer-permissions' import getMeta from '@/utils/meta' +import { useEditorContext } from '@/shared/context/editor-context' const dispatchReviewPanelEvent = (type: string, payload?: any) => { window.dispatchEvent( @@ -151,6 +152,7 @@ function useReviewPanelState(): ReviewPanel.ReviewPanelState { const permissions = usePermissionsContext() const { showGenericMessageModal } = useModalsContext() const addCommentEmitter = useScopeEventEmitter('comment:start_adding') + const hasViewerPermissions = useViewerPermissions() const layoutToLeft = useLayoutToLeft('.ide-react-editor-panel') const [subView, setSubView] = @@ -418,7 +420,7 @@ function useReviewPanelState(): ReviewPanel.ReviewPanelState { } if (!users[change.metadata.user_id]) { - if (!isRestrictedTokenMember) { + if (!(isRestrictedTokenMember || hasViewerPermissions)) { refreshChangeUsers(change.metadata.user_id) } } @@ -426,7 +428,10 @@ function useReviewPanelState(): ReviewPanel.ReviewPanelState { let localResolvedThreadIds = resolvedThreadIds - if (!isRestrictedTokenMember && rangesTracker.comments.length > 0) { + if ( + !(isRestrictedTokenMember || hasViewerPermissions) && + rangesTracker.comments.length > 0 + ) { const threadsLoadResult = await ensureThreadsAreLoaded() if (threadsLoadResult?.resolvedThreadIds) { localResolvedThreadIds = threadsLoadResult.resolvedThreadIds @@ -483,13 +488,14 @@ function useReviewPanelState(): ReviewPanel.ReviewPanelState { getChangeTracker, getDocEntries, getDocResolvedComments, - isRestrictedTokenMember, refreshChangeUsers, resolvedThreadIds, users, ensureThreadsAreLoaded, loadingThreads, setLoadingThreads, + hasViewerPermissions, + isRestrictedTokenMember, ] ) diff --git a/services/web/frontend/js/features/source-editor/components/codemirror-editor.tsx b/services/web/frontend/js/features/source-editor/components/codemirror-editor.tsx index f58396f040..c7f15f7efd 100644 --- a/services/web/frontend/js/features/source-editor/components/codemirror-editor.tsx +++ b/services/web/frontend/js/features/source-editor/components/codemirror-editor.tsx @@ -19,6 +19,7 @@ import { dispatchTimer } from '../../../infrastructure/cm6-performance' import importOverleafModules from '../../../../macros/import-overleaf-module.macro' import { FigureModal } from './figure-modal/figure-modal' import ReviewPanel from './review-panel/review-panel' +import useViewerPermissions from '@/shared/hooks/use-viewer-permissions' const sourceEditorComponents = importOverleafModules( 'sourceEditorComponents' @@ -36,6 +37,8 @@ function CodeMirrorEditor() { const isMounted = useIsMounted() + const shouldShowReviewPanel = !useViewerPermissions() + // create the view using the initial state and intercept transactions const viewRef = useRef(null) if (viewRef.current === null) { @@ -69,7 +72,8 @@ function CodeMirrorEditor() { ) )} - + + {shouldShowReviewPanel && } {sourceEditorComponents.map( ({ import: { default: Component }, path }) => ( diff --git a/services/web/frontend/js/shared/hooks/use-viewer-permissions.ts b/services/web/frontend/js/shared/hooks/use-viewer-permissions.ts new file mode 100644 index 0000000000..aca2fede22 --- /dev/null +++ b/services/web/frontend/js/shared/hooks/use-viewer-permissions.ts @@ -0,0 +1,12 @@ +import { useEditorContext } from '../context/editor-context' +import getMeta from '@/utils/meta' + +function useViewerPermissions() { + const { permissionsLevel } = useEditorContext() + + const hasViewerPermissions = + getMeta('ol-linkSharingWarning') && permissionsLevel === 'readOnly' + return hasViewerPermissions +} + +export default useViewerPermissions diff --git a/services/web/test/frontend/features/editor-navigation-toolbar/components/toolbar-header.test.jsx b/services/web/test/frontend/features/editor-navigation-toolbar/components/toolbar-header.test.jsx index da8d601621..f01e317993 100644 --- a/services/web/test/frontend/features/editor-navigation-toolbar/components/toolbar-header.test.jsx +++ b/services/web/test/frontend/features/editor-navigation-toolbar/components/toolbar-header.test.jsx @@ -17,6 +17,7 @@ describe('', function () { renameProject: () => {}, openShareModal: () => {}, hasPublishPermissions: true, + chatVisible: true, trackChangesVisible: true, handleChangeLayout: () => {}, pdfLayout: 'sideBySide', @@ -83,10 +84,10 @@ describe('', function () { screen.getByText('Chat') }) - it('is not displayed when "isRestrictedTokenMember" prop is set to true', function () { + it('is not displayed when "chatVisible" prop is set to false', function () { const props = { ...defaultProps, - isRestrictedTokenMember: true, + chatVisible: false, } renderWithEditorContext() expect(screen.queryByText('Chat')).to.not.exist