From faff4f1d7e10d5b51061fa38bc4e8ffb77d8cb74 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Thu, 28 Nov 2024 09:43:28 +0100 Subject: [PATCH] Merge pull request #22178 from overleaf/jpa-file-view-hash-2 [web] migrate file-view to download from history-v1 (via web) 2/2 GitOrigin-RevId: 93172cbbd2d19a55bb27f5b0ca0b494f815a3632 --- .../contexts/file-tree-actionable.tsx | 9 +--- .../file-tree/contexts/file-tree-path.tsx | 7 +--- .../js/features/file-tree/util/path.ts | 7 +--- .../file-view/components/file-view-header.tsx | 8 +--- .../file-view/components/file-view-image.tsx | 8 +--- .../file-view/components/file-view-text.tsx | 38 +++++++---------- .../stories/file-view/file-view.stories.jsx | 41 ++++++++----------- .../features/file-tree/util/path.test.ts | 17 +------- .../components/file-view-text.test.jsx | 4 +- .../file-view/components/file-view.test.jsx | 4 +- 10 files changed, 46 insertions(+), 97 deletions(-) diff --git a/services/web/frontend/js/features/file-tree/contexts/file-tree-actionable.tsx b/services/web/frontend/js/features/file-tree/contexts/file-tree-actionable.tsx index 7fe0d7acdb..115bda9dc2 100644 --- a/services/web/frontend/js/features/file-tree/contexts/file-tree-actionable.tsx +++ b/services/web/frontend/js/features/file-tree/contexts/file-tree-actionable.tsx @@ -33,7 +33,6 @@ import { } from '../errors' import { Folder } from '../../../../../types/folder' import { useReferencesContext } from '@/features/ide-react/context/references-context' -import { useSnapshotContext } from '@/features/ide-react/context/snapshot-context' type DroppedFile = File & { relativePath?: string @@ -221,7 +220,6 @@ export const FileTreeActionableProvider: FC = ({ children }) => { const { _id: projectId } = useProjectContext() const { fileTreeReadOnly } = useFileTreeData() const { indexAllReferences } = useReferencesContext() - const { fileTreeFromHistory } = useSnapshotContext() const [state, dispatch] = useReducer( fileTreeReadOnly @@ -494,17 +492,14 @@ export const FileTreeActionableProvider: FC = ({ children }) => { const selectedEntity = findInTree(fileTreeData, selectedEntityId) if (selectedEntity?.type === 'fileRef') { - if (fileTreeFromHistory) { - return `/project/${projectId}/blob/${selectedEntity.entity.hash}` - } - return `/project/${projectId}/file/${selectedEntityId}` + return `/project/${projectId}/blob/${selectedEntity.entity.hash}?fallback=${selectedEntityId}` } if (selectedEntity?.type === 'doc') { return `/project/${projectId}/doc/${selectedEntityId}/download` } } - }, [fileTreeData, projectId, selectedEntityIds, fileTreeFromHistory]) + }, [fileTreeData, projectId, selectedEntityIds]) // TODO: wrap in useMemo const value = { diff --git a/services/web/frontend/js/features/file-tree/contexts/file-tree-path.tsx b/services/web/frontend/js/features/file-tree/contexts/file-tree-path.tsx index 8ff7e87d61..593d51c6e1 100644 --- a/services/web/frontend/js/features/file-tree/contexts/file-tree-path.tsx +++ b/services/web/frontend/js/features/file-tree/contexts/file-tree-path.tsx @@ -10,7 +10,6 @@ import { pathInFolder, } from '@/features/file-tree/util/path' import { PreviewPath } from '../../../../../types/preview-path' -import { useSnapshotContext } from '@/features/ide-react/context/snapshot-context' type FileTreePathContextValue = { pathInFolder: (id: string) => string | null @@ -25,7 +24,6 @@ export const FileTreePathContext = createContext< export const FileTreePathProvider: FC = ({ children }) => { const { fileTreeData }: { fileTreeData: Folder } = useFileTreeData() - const { fileTreeFromHistory } = useSnapshotContext() const projectId = getMeta('ol-project_id') const pathInFileTree = useCallback( @@ -39,9 +37,8 @@ export const FileTreePathProvider: FC = ({ children }) => { ) const previewByPathInFileTree = useCallback( - (path: string) => - previewByPath(fileTreeData, projectId, path, fileTreeFromHistory), - [fileTreeData, projectId, fileTreeFromHistory] + (path: string) => previewByPath(fileTreeData, projectId, path), + [fileTreeData, projectId] ) const dirnameInFileTree = useCallback( diff --git a/services/web/frontend/js/features/file-tree/util/path.ts b/services/web/frontend/js/features/file-tree/util/path.ts index 0a63240a33..d90e824a57 100644 --- a/services/web/frontend/js/features/file-tree/util/path.ts +++ b/services/web/frontend/js/features/file-tree/util/path.ts @@ -106,8 +106,7 @@ export function findEntityByPath( export function previewByPath( folder: Folder, projectId: string, - path: string, - fileTreeFromHistory: boolean + path: string ): PreviewPath | null { for (const suffix of [ '', @@ -125,9 +124,7 @@ export function previewByPath( if (result?.type === 'fileRef') { const { name, _id: id, hash } = result.entity return { - url: fileTreeFromHistory - ? `/project/${projectId}/blob/${hash}` - : `/project/${projectId}/file/${id}`, + url: `/project/${projectId}/blob/${hash}?fallback=${id}`, extension: name.slice(name.lastIndexOf('.') + 1), } } diff --git a/services/web/frontend/js/features/file-view/components/file-view-header.tsx b/services/web/frontend/js/features/file-view/components/file-view-header.tsx index 04ff106db7..302037e38d 100644 --- a/services/web/frontend/js/features/file-view/components/file-view-header.tsx +++ b/services/web/frontend/js/features/file-view/components/file-view-header.tsx @@ -12,7 +12,6 @@ import { LinkedFileIcon } from './file-view-icons' import { BinaryFile, hasProvider, LinkedFile } from '../types/binary-file' import FileViewRefreshButton from './file-view-refresh-button' import FileViewRefreshError from './file-view-refresh-error' -import { useSnapshotContext } from '@/features/ide-react/context/snapshot-context' import MaterialIcon from '@/shared/components/material-icon' import BootstrapVersionSwitcher from '@/features/ui/components/bootstrap-5/bootstrap-version-switcher' import OLButton from '@/features/ui/components/ol/ol-button' @@ -53,7 +52,6 @@ type FileViewHeaderProps = { export default function FileViewHeader({ file }: FileViewHeaderProps) { const { _id: projectId } = useProjectContext() const { fileTreeReadOnly } = useFileTreeData() - const { fileTreeFromHistory } = useSnapshotContext() const { t } = useTranslation() const [refreshError, setRefreshError] = useState>(null) @@ -86,11 +84,7 @@ export default function FileViewHeader({ file }: FileViewHeaderProps) { } diff --git a/services/web/frontend/js/features/file-view/components/file-view-image.tsx b/services/web/frontend/js/features/file-view/components/file-view-image.tsx index 8ad6fb7f38..990fb69add 100644 --- a/services/web/frontend/js/features/file-view/components/file-view-image.tsx +++ b/services/web/frontend/js/features/file-view/components/file-view-image.tsx @@ -1,5 +1,4 @@ import { useProjectContext } from '../../../shared/context/project-context' -import { useSnapshotContext } from '@/features/ide-react/context/snapshot-context' import { BinaryFile } from '@/features/file-view/types/binary-file' export default function FileViewImage({ @@ -12,15 +11,10 @@ export default function FileViewImage({ onError: () => void }) { const { _id: projectId } = useProjectContext() - const { fileTreeFromHistory } = useSnapshotContext() return ( {file.name} void }) { const { _id: projectId } = useProjectContext() - const { fileTreeFromHistory } = useSnapshotContext() const [textPreview, setTextPreview] = useState('') const [shouldShowDots, setShouldShowDots] = useState(false) @@ -30,9 +28,7 @@ export default function FileViewText({ if (inFlight) { return } - let path = fileTreeFromHistory - ? `/project/${projectId}/blob/${file.hash}` - : `/project/${projectId}/file/${file.id}` + const path = `/project/${projectId}/blob/${file.hash}?fallback=${file.id}` const fetchContentLengthTimeout = setTimeout( () => fetchContentLengthController.abort(), 10000 @@ -45,32 +41,27 @@ export default function FileViewText({ }) .then(fileSize => { let truncated = false - let maxSize = null + const headers = new Headers() if (fileSize && Number(fileSize) > MAX_FILE_SIZE) { truncated = true - maxSize = MAX_FILE_SIZE - } - - if (maxSize != null) { - path += `?range=0-${maxSize}` + headers.set('Range', `bytes=0-${MAX_FILE_SIZE}`) } fetchDataTimeout = window.setTimeout( () => fetchDataController.abort(), 60000 ) - return fetch(path, { signal: fetchDataController.signal }).then( - response => { - return response.text().then(text => { - if (truncated) { - text = text.replace(/\n.*$/, '') - } + const signal = fetchDataController.signal + return fetch(path, { signal, headers }).then(response => { + return response.text().then(text => { + if (truncated) { + text = text.replace(/\n.*$/, '') + } - setTextPreview(text) - onLoad() - setShouldShowDots(truncated) - }) - } - ) + setTextPreview(text) + onLoad() + setShouldShowDots(truncated) + }) + }) }) .catch(err => { debugConsole.error('Error fetching file contents', err) @@ -82,7 +73,6 @@ export default function FileViewText({ clearTimeout(fetchDataTimeout) }) }, [ - fileTreeFromHistory, projectId, file.id, file.hash, diff --git a/services/web/frontend/stories/file-view/file-view.stories.jsx b/services/web/frontend/stories/file-view/file-view.stories.jsx index 86a8b751b0..db51aef50c 100644 --- a/services/web/frontend/stories/file-view/file-view.stories.jsx +++ b/services/web/frontend/stories/file-view/file-view.stories.jsx @@ -21,7 +21,7 @@ extra parameters or packages included. const setupFetchMock = fetchMock => { return fetchMock - .head('express:/project/:project_id/file/:file_id', { + .head('express:/project/:project_id/blob/:hash', { status: 201, headers: { 'Content-Length': 10000 }, }) @@ -41,10 +41,9 @@ const fileData = { export const FileFromUrl = args => { useFetchMock(fetchMock => - setupFetchMock(fetchMock).get( - 'express:/project/:project_id/file/:file_id', - { body: bodies.latex } - ) + setupFetchMock(fetchMock).get('express:/project/:project_id/blob/:hash', { + body: bodies.latex, + }) ) return @@ -61,10 +60,9 @@ FileFromUrl.args = { export const FileFromProjectWithLinkableProjectId = args => { useFetchMock(fetchMock => - setupFetchMock(fetchMock).get( - 'express:/project/:project_id/file/:file_id', - { body: bodies.latex } - ) + setupFetchMock(fetchMock).get('express:/project/:project_id/blob/:hash', { + body: bodies.latex, + }) ) return @@ -82,10 +80,9 @@ FileFromProjectWithLinkableProjectId.args = { export const FileFromProjectWithoutLinkableProjectId = args => { useFetchMock(fetchMock => - setupFetchMock(fetchMock).get( - 'express:/project/:project_id/file/:file_id', - { body: bodies.latex } - ) + setupFetchMock(fetchMock).get('express:/project/:project_id/blob/:hash', { + body: bodies.latex, + }) ) return @@ -103,10 +100,9 @@ FileFromProjectWithoutLinkableProjectId.args = { export const FileFromProjectOutputWithLinkableProject = args => { useFetchMock(fetchMock => - setupFetchMock(fetchMock).get( - 'express:/project/:project_id/file/:file_id', - { body: bodies.latex } - ) + setupFetchMock(fetchMock).get('express:/project/:project_id/blob/:hash', { + body: bodies.latex, + }) ) return @@ -124,10 +120,9 @@ FileFromProjectOutputWithLinkableProject.args = { export const FileFromProjectOutputWithoutLinkableProjectId = args => { useFetchMock(fetchMock => - setupFetchMock(fetchMock).get( - 'express:/project/:project_id/file/:file_id', - { body: bodies.latex } - ) + setupFetchMock(fetchMock).get('express:/project/:project_id/blob/:hash', { + body: bodies.latex, + }) ) return @@ -165,7 +160,7 @@ ImageFile.args = { export const TextFile = args => { useFetchMock(fetchMock => setupFetchMock(fetchMock).get( - 'express:/project/:project_id/file/:file_id', + 'express:/project/:project_id/blob/:hash', { body: bodies.text }, { overwriteRoutes: true } ) @@ -187,7 +182,7 @@ TextFile.args = { export const UploadedFile = args => { useFetchMock(fetchMock => setupFetchMock(fetchMock).head( - 'express:/project/:project_id/file/:file_id', + 'express:/project/:project_id/blob/:hash', { status: 500 }, { overwriteRoutes: true } ) diff --git a/services/web/test/frontend/features/file-tree/util/path.test.ts b/services/web/test/frontend/features/file-tree/util/path.test.ts index 8b52f176e3..e820c14a7c 100644 --- a/services/web/test/frontend/features/file-tree/util/path.test.ts +++ b/services/web/test/frontend/features/file-tree/util/path.test.ts @@ -151,23 +151,10 @@ describe('Path utils', function () { const preview = previewByPath( rootFolder, 'test-project-id', - 'test-folder/example.png', - false + 'test-folder/example.png' ) expect(preview).to.deep.equal({ - url: '/project/test-project-id/file/test-file-in-folder', - extension: 'png', - }) - }) - it('returns handles history file-tree', function () { - const preview = previewByPath( - rootFolder, - 'test-project-id', - 'test-folder/example.png', - true - ) - expect(preview).to.deep.equal({ - url: '/project/test-project-id/blob/42', + url: '/project/test-project-id/blob/42?fallback=test-file-in-folder', extension: 'png', }) }) diff --git a/services/web/test/frontend/features/file-view/components/file-view-text.test.jsx b/services/web/test/frontend/features/file-view/components/file-view-text.test.jsx index 36fb9325f6..ec19f57ecf 100644 --- a/services/web/test/frontend/features/file-view/components/file-view-text.test.jsx +++ b/services/web/test/frontend/features/file-view/components/file-view-text.test.jsx @@ -23,12 +23,12 @@ describe('', function () { }) it('renders a text view', async function () { - fetchMock.head('express:/project/:project_id/file/:file_id', { + fetchMock.head('express:/project/:project_id/blob/:hash', { status: 201, headers: { 'Content-Length': 10000 }, }) fetchMock.get( - 'express:/project/:project_id/file/:file_id', + 'express:/project/:project_id/blob/:hash', 'Text file content' ) diff --git a/services/web/test/frontend/features/file-view/components/file-view.test.jsx b/services/web/test/frontend/features/file-view/components/file-view.test.jsx index 638b5d40ea..390505dc16 100644 --- a/services/web/test/frontend/features/file-view/components/file-view.test.jsx +++ b/services/web/test/frontend/features/file-view/components/file-view.test.jsx @@ -36,12 +36,12 @@ describe('', function () { describe('for a text file', function () { it('shows a loading indicator while the file is loading', async function () { - fetchMock.head('express:/project/:project_id/file/:file_id', { + fetchMock.head('express:/project/:project_id/blob/:hash', { status: 201, headers: { 'Content-Length': 10000 }, }) fetchMock.get( - 'express:/project/:project_id/file/:file_id', + 'express:/project/:project_id/blob/:hash', 'Text file content' )