Merge pull request #3561 from overleaf/ta-file-tree-move-fixes

[ReactFileTree] Move & Rename Validation Fixes

GitOrigin-RevId: 30c2ff17454246c82a03d0b869a810048c9f5dad

services/web/frontend/extracted-translation-keys.json

@@ -24,6 +24,8 @@
   "fast",
   "file_outline",
   "file_already_exists",
+  "file_already_exists_in_this_location",
+  "blocked_filename",
   "files_cannot_include_invalid_characters",
   "find_out_more_about_the_file_outline",
   "first_error_popup_label",

services/web/frontend/js/features/file-tree/components/modals/file-tree-modal-error.js

@@ -1,18 +1,25 @@
 import React from 'react'
 
 import { Button, Modal } from 'react-bootstrap'
-import { useTranslation } from 'react-i18next'
+import { Trans, useTranslation } from 'react-i18next'
 
 import { useFileTreeActionable } from '../../contexts/file-tree-actionable'
 
-import { InvalidFilenameError, DuplicateFilenameError } from '../../errors'
+import {
+  InvalidFilenameError,
+  BlockedFilenameError,
+  DuplicateFilenameError,
+  DuplicateFilenameMoveError
+} from '../../errors'
 
 function FileTreeModalError() {
   const { t } = useTranslation()
 
-  const { isRenaming, cancel, error } = useFileTreeActionable()
+  const { isRenaming, isMoving, cancel, error } = useFileTreeActionable()
 
-  if (!isRenaming || !error) return null // the modal will not be rendered; return early
+  // the modal will not be rendered; return early
+  if (!error) return null
+  if (!isRenaming && !isMoving) return null
 
   function handleHide() {
     cancel()
@@ -21,8 +28,10 @@ function FileTreeModalError() {
   function errorTitle() {
     switch (error.constructor) {
       case DuplicateFilenameError:
+      case DuplicateFilenameMoveError:
         return t('duplicate_file')
       case InvalidFilenameError:
+      case BlockedFilenameError:
         return t('invalid_file_name')
       default:
         return t('error')
@@ -33,8 +42,18 @@ function FileTreeModalError() {
     switch (error.constructor) {
       case DuplicateFilenameError:
         return t('file_already_exists')
+      case DuplicateFilenameMoveError:
+        return (
+          <Trans
+            i18nKey="file_already_exists_in_this_location"
+            components={[<strong />]} // eslint-disable-line react/jsx-key
+            values={{ fileName: error.entityName }}
+          />
+        )
       case InvalidFilenameError:
         return t('files_cannot_include_invalid_characters')
+      case BlockedFilenameError:
+        return t('blocked_filename')
       default:
         return t('generic_something_went_wrong')
     }

services/web/frontend/js/features/file-tree/contexts/file-tree-actionable.js

@@ -11,13 +11,18 @@ import {
 } from '../util/sync-mutation'
 import { findInTreeOrThrow } from '../util/find-in-tree'
 import { isNameUniqueInFolder } from '../util/is-name-unique-in-folder'
-import { isCleanFilename } from '../util/safe-path'
+import { isBlockedFilename, isCleanFilename } from '../util/safe-path'
 
 import { FileTreeMainContext } from './file-tree-main'
 import { useFileTreeMutable } from './file-tree-mutable'
 import { useFileTreeSelectable } from './file-tree-selectable'
 
-import { InvalidFilenameError, DuplicateFilenameError } from '../errors'
+import {
+  InvalidFilenameError,
+  BlockedFilenameError,
+  DuplicateFilenameError,
+  DuplicateFilenameMoveError
+} from '../errors'
 
 const FileTreeActionableContext = createContext()
 
@@ -27,6 +32,7 @@ const ACTION_TYPES = {
   DELETING: 'DELETING',
   START_CREATE_FOLDER: 'START_CREATE_FOLDER',
   CREATING_FOLDER: 'CREATING_FOLDER',
+  MOVING: 'MOVING',
   CANCEL: 'CANCEL',
   CLEAR: 'CLEAR',
   ERROR: 'ERROR'
@@ -36,6 +42,7 @@ const defaultState = {
   isDeleting: false,
   isRenaming: false,
   isCreatingFolder: false,
+  isMoving: false,
   inFlight: false,
   actionedEntities: null,
   error: null
@@ -68,6 +75,12 @@ function fileTreeActionableReducer(state, action) {
         inFlight: true,
         actionedEntities: state.actionedEntities
       }
+    case ACTION_TYPES.MOVING:
+      return {
+        ...defaultState,
+        isMoving: true,
+        inFlight: true
+      }
     case ACTION_TYPES.CLEAR:
       return { ...defaultState }
     case ACTION_TYPES.CANCEL:
@@ -107,6 +120,7 @@ export function useFileTreeActionable() {
   const {
     isDeleting,
     isRenaming,
+    isMoving,
     isCreatingFolder,
     inFlight,
     error,
@@ -130,18 +144,9 @@ export function useFileTreeActionable() {
     if (newName === oldName) {
       return dispatch({ type: ACTION_TYPES.CLEAR })
     }
-    let error
-    // check valid name
-    if (!isCleanFilename(newName)) {
-      error = new InvalidFilenameError()
-      return dispatch({ type: ACTION_TYPES.ERROR, error })
-    }
 
-    // check for duplicates
-    if (!isNameUniqueInFolder(fileTreeData, found.parentFolderId, newName)) {
-      error = new DuplicateFilenameError()
-      return dispatch({ type: ACTION_TYPES.ERROR, error })
-    }
+    const error = validateRename(fileTreeData, found, newName)
+    if (error) return dispatch({ type: ACTION_TYPES.ERROR, error })
 
     dispatch({ type: ACTION_TYPES.CLEAR })
     dispatchRename(selectedEntityId, newName)
@@ -191,14 +196,35 @@ export function useFileTreeActionable() {
 
   // moves entities. Tree is updated immediately and data are sync'd after.
   function finishMoving(toFolderId, draggedEntityIds) {
-    draggedEntityIds.forEach(selectedEntityId => {
-      dispatchMove(selectedEntityId, toFolderId)
-    })
+    dispatch({ type: ACTION_TYPES.MOVING })
 
-    return mapSeries(Array.from(draggedEntityIds), id => {
-      const found = findInTreeOrThrow(fileTreeData, id)
-      return syncMove(projectId, found.type, found.entity._id, toFolderId)
-    })
+    // find entities and filter out no-ops
+    const founds = Array.from(draggedEntityIds)
+      .map(draggedEntityId => findInTreeOrThrow(fileTreeData, draggedEntityId))
+      .filter(found => found.parentFolderId !== toFolderId)
+
+    // make sure all entities can be moved, return early otherwise
+    const isMoveToRoot = toFolderId === fileTreeData._id
+    const validationError = founds
+      .map(found => validateMove(fileTreeData, toFolderId, found, isMoveToRoot))
+      .find(error => error)
+    if (validationError) {
+      return dispatch({ type: ACTION_TYPES.ERROR, error: validationError })
+    }
+
+    // dispatch moves immediately
+    founds.forEach(found => dispatchMove(found.entity._id, toFolderId))
+
+    // sync dispatched moves after
+    return mapSeries(founds, found =>
+      syncMove(projectId, found.type, found.entity._id, toFolderId)
+    )
+      .then(() => {
+        dispatch({ type: ACTION_TYPES.CLEAR })
+      })
+      .catch(error => {
+        dispatch({ type: ACTION_TYPES.ERROR, error })
+      })
   }
 
   function startCreatingFolder() {
@@ -308,6 +334,7 @@ export function useFileTreeActionable() {
     canRename: selectedEntityIds.size === 1,
     canCreate: selectedEntityIds.size < 2,
     isDeleting,
+    isMoving,
     isRenaming,
     isCreatingFolder,
     inFlight,
@@ -339,3 +366,32 @@ function getSelectedParentFolderId(fileTreeData, selectedEntityIds) {
   const found = findInTreeOrThrow(fileTreeData, selectedEntityId)
   return found.type === 'folder' ? found.entity._id : found.parentFolderId
 }
+
+function validateRename(fileTreeData, found, newName) {
+  if (!isCleanFilename(newName)) {
+    return new InvalidFilenameError()
+  }
+
+  if (!isNameUniqueInFolder(fileTreeData, found.parentFolderId, newName)) {
+    return new DuplicateFilenameError()
+  }
+
+  const isTopLevel = found.path.length === 1
+  const isFolder = found.type === 'folder'
+  if (isTopLevel && !isFolder && isBlockedFilename(newName)) {
+    return new BlockedFilenameError()
+  }
+}
+
+function validateMove(fileTreeData, toFolderId, found, isMoveToRoot) {
+  if (!isNameUniqueInFolder(fileTreeData, toFolderId, found.entity.name)) {
+    const error = new DuplicateFilenameMoveError()
+    error.entityName = found.entity.name
+    return error
+  }
+
+  const isFolder = found.type === 'folder'
+  if (isMoveToRoot && !isFolder && isBlockedFilename(found.entity.name)) {
+    return new BlockedFilenameError()
+  }
+}

services/web/frontend/js/features/file-tree/errors.js

@@ -4,8 +4,20 @@ export class InvalidFilenameError extends Error {
   }
 }
 
+export class BlockedFilenameError extends Error {
+  constructor() {
+    super('blocked filename')
+  }
+}
+
 export class DuplicateFilenameError extends Error {
   constructor() {
     super('duplicate filename')
   }
 }
+
+export class DuplicateFilenameMoveError extends Error {
+  constructor() {
+    super('duplicate filename on move')
+  }
+}

services/web/frontend/js/features/file-tree/util/safe-path.js

@@ -76,6 +76,34 @@ export function isCleanFilename(filename) {
   )
 }
 
+export function isBlockedFilename(filename) {
+  return BLOCKEDFILE_RX.test(filename)
+}
+
+// returns whether a full path is 'clean' - e.g. is a full or relative path
+// that points to a file, and each element passes the rules in 'isCleanFilename'
+export function isCleanPath(path) {
+  const elements = path.split('/')
+
+  const lastElementIsEmpty = elements[elements.length - 1].length === 0
+  if (lastElementIsEmpty) {
+    return false
+  }
+
+  for (let element of Array.from(elements)) {
+    if (element.length > 0 && !isCleanFilename(element)) {
+      return false
+    }
+  }
+
+  // check for a top-level reserved name
+  if (BLOCKEDFILE_RX.test(path.replace(/^\/?/, ''))) {
+    return false
+  } // remove leading slash if present
+
+  return true
+}
+
 export function isAllowedLength(pathname) {
   return pathname.length > 0 && pathname.length <= MAX_PATH
 }

services/web/locales/en.json

@@ -93,7 +93,8 @@
   "confirmation_token_invalid": "Sorry, your confirmation token is invalid or has expired. Please request a new email confirmation link.",
   "confirmation_link_broken": "Sorry, something is wrong with your confirmation link. Please try copy and pasting the link from the bottom of your confirmation email.",
   "duplicate_file": "Duplicate File",
-  "file_already_exists_in_this_location": "An item named __fileName__ already exists in this location. If you wish to move this file, rename or remove the conflicting file and try again.",
+  "file_already_exists_in_this_location": "An item named <0>__fileName__</0> already exists in this location. If you wish to move this file, rename or remove the conflicting file and try again.",
+  "blocked_filename": "This file name is blocked.",
   "group_full": "This group is already full",
   "no_selection_select_file": "Currently, no file is selected. Please select a file from the file tree.",
   "no_selection_create_new_file": "Your project is currently empty. Please create a new file.",

services/web/test/frontend/features/file-tree/flows/rename-entity.test.js

@@ -150,6 +150,17 @@ describe('FileTree Rename Entity Flow', function() {
     })
   })
 
+  it('shows error modal on blocked filename', async function() {
+    const input = initItemRename('a.tex')
+    fireEvent.change(input, { target: { value: 'prototype' } })
+    fireEvent.keyDown(input, { key: 'Enter' })
+
+    await screen.findByRole('alert', {
+      name: 'This file name is blocked.',
+      hidden: true
+    })
+  })
+
   describe('via socket event', function() {
     it('renames doc', function() {
       screen.getByRole('treeitem', { name: 'a.tex' })