diff --git a/services/project-history/app/js/FileTreeDiffGenerator.js b/services/project-history/app/js/FileTreeDiffGenerator.js index 49a87d3821..5937b7a70e 100644 --- a/services/project-history/app/js/FileTreeDiffGenerator.js +++ b/services/project-history/app/js/FileTreeDiffGenerator.js @@ -54,21 +54,20 @@ function _getInitialDiffSnapshot(chunk, fromVersion) { // with nothing in the diff marked as changed. // Use a bare object to protect against reserved names. const diff = Object.create(null) - const pathnames = _getInitialPathnames(chunk, fromVersion) - for (const pathname of Array.from(pathnames)) { - diff[pathname] = { pathname } + const files = _getInitialFiles(chunk, fromVersion) + for (const [pathname, file] of Object.entries(files)) { + diff[pathname] = { pathname, editable: file.isEditable() } } return diff } -function _getInitialPathnames(chunk, fromVersion) { +function _getInitialFiles(chunk, fromVersion) { const snapshot = chunk.getSnapshot() const changes = chunk .getChanges() .slice(0, fromVersion - chunk.getStartVersion()) snapshot.applyAll(changes) - const pathnames = snapshot.getFilePathnames() - return pathnames + return snapshot.fileMap.files } function _applyAddFileToDiff(diff, operation) { @@ -88,6 +87,7 @@ function _applyAddFileToDiff(diff, operation) { return (diff[operation.pathname] = { pathname: operation.pathname, operation: 'added', + editable: operation.file.isEditable(), }) } } diff --git a/services/project-history/test/acceptance/js/FileTreeDiffTests.js b/services/project-history/test/acceptance/js/FileTreeDiffTests.js index 0b947f78bb..0e8f897fef 100644 --- a/services/project-history/test/acceptance/js/FileTreeDiffTests.js +++ b/services/project-history/test/acceptance/js/FileTreeDiffTests.js @@ -123,6 +123,7 @@ describe('FileTree Diffs', function () { { file: { hash: sha('new-sha'), + stringLength: 42, }, pathname: 'added.tex', }, @@ -155,15 +156,18 @@ describe('FileTree Diffs', function () { pathname: 'deleted.tex', operation: 'removed', deletedAtV: 5, + editable: true, }, { newPathname: 'newName.tex', pathname: 'renamed.tex', operation: 'renamed', + editable: true, }, { pathname: 'added.tex', operation: 'added', + editable: true, }, ], }) @@ -269,6 +273,7 @@ describe('FileTree Diffs', function () { { file: { hash: sha('new-sha'), + stringLength: 42, }, pathname: 'added.tex', }, @@ -313,20 +318,24 @@ describe('FileTree Diffs', function () { }, { pathname: 'baz.tex', + editable: true, }, { pathname: 'deleted.tex', operation: 'removed', deletedAtV: 4, + editable: true, }, { newPathname: 'newName.tex', pathname: 'renamed.tex', operation: 'renamed', + editable: true, }, { pathname: 'added.tex', operation: 'added', + editable: true, }, ], }) @@ -391,6 +400,7 @@ describe('FileTree Diffs', function () { newPathname: 'three.tex', pathname: 'one.tex', operation: 'renamed', + editable: true, }, ], }) @@ -455,6 +465,7 @@ describe('FileTree Diffs', function () { diff: [ { pathname: 'one.tex', + editable: true, }, ], }) @@ -523,6 +534,7 @@ describe('FileTree Diffs', function () { pathname: 'two.tex', newPathname: 'one.tex', operation: 'renamed', + editable: true, }, ], }) @@ -547,6 +559,7 @@ describe('FileTree Diffs', function () { pathname: 'one.tex', file: { hash: sha('mock-sha'), + stringLength: 42, }, }, ], @@ -583,6 +596,7 @@ describe('FileTree Diffs', function () { { pathname: 'two.tex', operation: 'added', + editable: true, }, ], }) diff --git a/services/web/frontend/js/features/history/services/types/file.ts b/services/web/frontend/js/features/history/services/types/file.ts index 3752f0b05c..4fb1330087 100644 --- a/services/web/frontend/js/features/history/services/types/file.ts +++ b/services/web/frontend/js/features/history/services/types/file.ts @@ -1,24 +1,30 @@ import { FileOperation } from './file-operation' -export interface FileUnchanged { +interface File { pathname: string } -export interface FileAdded extends FileUnchanged { +export interface FileWithEditable extends File { + editable: boolean +} + +export type FileUnchanged = FileWithEditable + +export interface FileAdded extends FileWithEditable { operation: Extract } -export interface FileRemoved extends FileUnchanged { +export interface FileRemoved extends FileWithEditable { operation: Extract newPathname?: string deletedAtV: number } -export interface FileEdited extends FileUnchanged { +export interface FileEdited extends File { operation: Extract } -export interface FileRenamed extends FileUnchanged { +export interface FileRenamed extends FileWithEditable { newPathname?: string oldPathname?: string operation: Extract diff --git a/services/web/frontend/js/features/history/utils/auto-select-file.ts b/services/web/frontend/js/features/history/utils/auto-select-file.ts index b23eaa7607..785231571e 100644 --- a/services/web/frontend/js/features/history/utils/auto-select-file.ts +++ b/services/web/frontend/js/features/history/utils/auto-select-file.ts @@ -1,12 +1,12 @@ -import _ from 'lodash' import type { Nullable } from '../../../../../types/utils' import type { FileDiff } from '../services/types/file' import type { FileOperation } from '../services/types/file-operation' import type { LoadedUpdate, Version } from '../services/types/update' -import { fileFinalPathname } from './file-diff' +import { fileFinalPathname, isFileEditable } from './file-diff' type FileWithOps = { pathname: FileDiff['pathname'] + editable: boolean operation: FileOperation } @@ -20,54 +20,70 @@ function getFilesWithOps( const filesWithOps: FileWithOps[] = [] if (updateForToV) { + const filesByPathname = new Map() + for (const file of files) { + const pathname = fileFinalPathname(file) + filesByPathname.set(pathname, file) + } + + const isEditable = (pathname: string) => { + const fileDiff = filesByPathname.get(pathname) + if (!fileDiff) { + return false + } + return isFileEditable(fileDiff) + } + for (const pathname of updateForToV.pathnames) { filesWithOps.push({ pathname, + editable: isEditable(pathname), operation: 'edited', }) } for (const op of updateForToV.project_ops) { - let fileWithOps: Nullable = null + let pathAndOp: Nullable> = + null if (op.add) { - fileWithOps = { + pathAndOp = { pathname: op.add.pathname, operation: 'added', } } else if (op.remove) { - fileWithOps = { + pathAndOp = { pathname: op.remove.pathname, operation: 'removed', } } else if (op.rename) { - fileWithOps = { + pathAndOp = { pathname: op.rename.newPathname, operation: 'renamed', } } - if (fileWithOps !== null) { - filesWithOps.push(fileWithOps) + if (pathAndOp !== null) { + filesWithOps.push({ + editable: isEditable(pathAndOp.pathname), + ...pathAndOp, + }) } } } return filesWithOps } else { - const filesWithOps = _.reduce( - files, - (curFilesWithOps, file) => { - if ('operation' in file) { - curFilesWithOps.push({ - pathname: file.pathname, - operation: file.operation, - }) - } - return curFilesWithOps - }, - [] - ) + const filesWithOps = files.reduce((curFilesWithOps, file) => { + if ('operation' in file) { + curFilesWithOps.push({ + pathname: file.pathname, + editable: isFileEditable(file), + operation: file.operation, + }) + } + return curFilesWithOps + }, []) return filesWithOps } @@ -86,44 +102,25 @@ export function autoSelectFile( comparing: boolean, updateForToV: LoadedUpdate | undefined ): FileDiff { - let fileToSelect: Nullable = null - const filesWithOps = getFilesWithOps(files, toV, comparing, updateForToV) for (const opType of orderedOpTypes) { - const fileWithMatchingOpType = _.find(filesWithOps, { - operation: opType, - }) + const fileWithMatchingOpType = filesWithOps.find( + file => file.operation === opType && file.editable + ) - if (fileWithMatchingOpType != null) { - fileToSelect = - _.find( - files, - file => fileFinalPathname(file) === fileWithMatchingOpType.pathname - ) ?? null - - break - } - } - - if (!fileToSelect) { - const mainFile = _.find(files, function (file) { - return /main\.tex$/.test(file.pathname) - }) - - if (mainFile) { - fileToSelect = mainFile - } else { - const anyTeXFile = _.find(files, function (file) { - return /\.tex$/.test(file.pathname) - }) - - if (anyTeXFile) { - fileToSelect = anyTeXFile - } else { - fileToSelect = files[0] + if (fileWithMatchingOpType) { + const fileToSelect = files.find( + file => fileFinalPathname(file) === fileWithMatchingOpType.pathname + ) + if (fileToSelect) { + return fileToSelect } } } - return fileToSelect + return ( + files.find(file => /main\.tex$/.test(file.pathname)) || + files.find(file => /\.tex$/.test(file.pathname)) || + files[0] + ) } diff --git a/services/web/frontend/js/features/history/utils/file-diff.ts b/services/web/frontend/js/features/history/utils/file-diff.ts index 072cceb3af..3d5febbb10 100644 --- a/services/web/frontend/js/features/history/utils/file-diff.ts +++ b/services/web/frontend/js/features/history/utils/file-diff.ts @@ -1,4 +1,9 @@ -import type { FileDiff, FileRemoved, FileRenamed } from '../services/types/file' +import type { + FileDiff, + FileRemoved, + FileRenamed, + FileWithEditable, +} from '../services/types/file' export function isFileRenamed(fileDiff: FileDiff): fileDiff is FileRenamed { return (fileDiff as FileRenamed).operation === 'renamed' @@ -8,6 +13,18 @@ export function isFileRemoved(fileDiff: FileDiff): fileDiff is FileRemoved { return (fileDiff as FileRemoved).operation === 'removed' } -export function fileFinalPathname(fileDiff: FileDiff) { - return isFileRenamed(fileDiff) ? fileDiff.newPathname : fileDiff.pathname +function isFileWithEditable(fileDiff: FileDiff): fileDiff is FileWithEditable { + return 'editable' in (fileDiff as FileWithEditable) +} + +export function isFileEditable(fileDiff: FileDiff) { + return isFileWithEditable(fileDiff) + ? fileDiff.editable + : fileDiff.operation === 'edited' +} + +export function fileFinalPathname(fileDiff: FileDiff) { + return ( + (isFileRenamed(fileDiff) ? fileDiff.newPathname : null) || fileDiff.pathname + ) } diff --git a/services/web/frontend/js/features/history/utils/file-tree.ts b/services/web/frontend/js/features/history/utils/file-tree.ts index 543f098b82..423621902c 100644 --- a/services/web/frontend/js/features/history/utils/file-tree.ts +++ b/services/web/frontend/js/features/history/utils/file-tree.ts @@ -1,6 +1,6 @@ import _ from 'lodash' import type { FileDiff, FileRenamed } from '../services/types/file' -import { isFileRemoved } from './file-diff' +import { isFileEditable, isFileRemoved } from './file-diff' export type FileTreeEntity = { name?: string @@ -37,6 +37,7 @@ export function reducePathsToTree( type: 'folder', children: [], pathname: pathPart, + editable: false, } currentFileTreeLocation.push(fileTreeEntity) @@ -68,17 +69,12 @@ export function fileTreeDiffToFileTreeData( if (file.type === 'file') { const deletedAtV = isFileRemoved(file) ? file.deletedAtV : undefined - let newDoc: HistoryDoc = { + const newDoc: HistoryDoc = { pathname: file.pathname ?? '', name: file.name ?? '', deletedAtV, - } - - if ('operation' in file) { - newDoc = { - ...newDoc, - operation: file.operation, - } + editable: isFileEditable(file), + operation: 'operation' in file ? file.operation : undefined, } docs.push(newDoc) @@ -115,5 +111,6 @@ export function renamePathnameKey(file: FileRenamed): FileRenamed { oldPathname: file.pathname, pathname: file.newPathname as string, operation: file.operation, + editable: file.editable, } } diff --git a/services/web/test/frontend/features/history/components/toolbar.spec.tsx b/services/web/test/frontend/features/history/components/toolbar.spec.tsx index 6173436039..aaea070251 100644 --- a/services/web/test/frontend/features/history/components/toolbar.spec.tsx +++ b/services/web/test/frontend/features/history/components/toolbar.spec.tsx @@ -43,13 +43,16 @@ describe('history toolbar', function () { }, { pathname: 'sample.bib', + editable: true, }, { pathname: 'frog.jpg', + editable: false, }, ], selectedFile: { pathname: 'main.tex', + editable: true, }, } @@ -83,18 +86,22 @@ describe('history toolbar', function () { { pathname: 'main.tex', operation: 'added', + editable: true, }, { pathname: 'sample.bib', operation: 'added', + editable: true, }, { pathname: 'frog.jpg', operation: 'added', + editable: false, }, ], selectedFile: { pathname: 'main.tex', + editable: true, }, } diff --git a/services/web/test/frontend/features/history/utils/auto-select-file.test.ts b/services/web/test/frontend/features/history/utils/auto-select-file.test.ts index 2293c339e7..7270e33361 100644 --- a/services/web/test/frontend/features/history/utils/auto-select-file.test.ts +++ b/services/web/test/frontend/features/history/utils/auto-select-file.test.ts @@ -23,18 +23,23 @@ describe('autoSelectFile', function () { const files: FileDiff[] = [ { pathname: 'main.tex', + editable: true, }, { pathname: 'sample.bib', + editable: true, }, { pathname: 'frog.jpg', + editable: false, }, { pathname: 'newfile5.tex', + editable: true, }, { pathname: 'newfolder1/newfolder2/newfile2.tex', + editable: true, }, { pathname: 'newfolder1/newfile10.tex', @@ -276,18 +281,22 @@ describe('autoSelectFile', function () { { pathname: 'main.tex', operation: 'added', + editable: true, }, { pathname: 'sample.bib', operation: 'added', + editable: true, }, { pathname: 'frog.jpg', operation: 'added', + editable: false, }, { pathname: 'newfile1.tex', operation: 'added', + editable: true, }, ] @@ -347,17 +356,21 @@ describe('autoSelectFile', function () { pathname: 'main.tex', operation: 'removed', deletedAtV: 6, + editable: true, }, { pathname: 'sample.bib', + editable: true, }, { pathname: 'main2.tex', operation: 'added', + editable: true, }, { pathname: 'main3.tex', operation: 'added', + editable: true, }, ] @@ -446,18 +459,22 @@ describe('autoSelectFile', function () { const files: FileDiff[] = [ { pathname: 'main.tex', + editable: true, }, { pathname: 'sample.bib', + editable: true, }, { pathname: 'frog.jpg', + editable: false, }, { pathname: 'newfolder/maybewillbedeleted.tex', newPathname: 'newfolder2/maybewillbedeleted.tex', operation: 'removed', deletedAtV: 10, + editable: true, }, ] @@ -617,12 +634,15 @@ describe('autoSelectFile', function () { const files: FileDiff[] = [ { pathname: 'certainly_not_main.tex', + editable: true, }, { pathname: 'newfile.tex', + editable: true, }, { pathname: 'file2.tex', + editable: true, }, ] @@ -725,11 +745,13 @@ describe('autoSelectFile', function () { const files: FileDiff[] = [ { pathname: 'main.tex', + editable: true, }, { pathname: 'original.bib', newPathname: 'new.bib', operation: 'renamed', + editable: true, }, ] @@ -792,5 +814,79 @@ describe('autoSelectFile', function () { expect(pathname).to.equal('new.bib') }) + + it('ignores binary file', function () { + const files: FileDiff[] = [ + { + pathname: 'frog.jpg', + editable: false, + operation: 'added', + }, + { + pathname: 'main.tex', + editable: true, + }, + { + pathname: 'sample.bib', + editable: true, + }, + ] + + const updates: LoadedUpdate[] = [ + { + fromV: 4, + toV: 7, + meta: { + users: historyUsers, + start_ts: 1680874742389, + end_ts: 1680874755552, + }, + labels: [], + pathnames: [], + project_ops: [ + { + add: { + pathname: 'frog.jpg', + }, + atV: 5, + }, + ], + }, + { + fromV: 0, + toV: 4, + meta: { + users: historyUsers, + start_ts: 1680861975947, + end_ts: 1680861988442, + }, + labels: [], + pathnames: [], + project_ops: [ + { + add: { + pathname: 'sample.bib', + }, + atV: 1, + }, + { + add: { + pathname: 'main.tex', + }, + atV: 0, + }, + ], + }, + ] + + const { pathname } = autoSelectFile( + files, + updates[0].toV, + comparing, + getUpdateForVersion(updates[0].toV, updates) + ) + + expect(pathname).to.equal('main.tex') + }) }) })