diff options
| -rw-r--r-- | apps/files/src/components/FileEntry/FileEntryName.vue | 70 | ||||
| -rw-r--r-- | apps/files/src/store/renaming.ts | 83 | ||||
| -rw-r--r-- | cypress/e2e/files/files-renaming.cy.ts | 30 |
3 files changed, 122 insertions, 61 deletions
diff --git a/apps/files/src/components/FileEntry/FileEntryName.vue b/apps/files/src/components/FileEntry/FileEntryName.vue index cf425127282..5236af0cfad 100644 --- a/apps/files/src/components/FileEntry/FileEntryName.vue +++ b/apps/files/src/components/FileEntry/FileEntryName.vue @@ -41,12 +41,9 @@ import type { FileAction, Node } from '@nextcloud/files' import type { PropType } from 'vue' -import axios, { isAxiosError } from '@nextcloud/axios' import { showError, showSuccess } from '@nextcloud/dialogs' -import { emit } from '@nextcloud/event-bus' import { FileType, NodeStatus } from '@nextcloud/files' import { translate as t } from '@nextcloud/l10n' -import { dirname } from '@nextcloud/paths' import { defineComponent, inject } from 'vue' import NcTextField from '@nextcloud/vue/dist/Components/NcTextField.js' @@ -245,66 +242,23 @@ export default defineComponent({ } const oldName = this.source.basename - const oldEncodedSource = this.source.encodedSource - if (oldName === newName) { - this.stopRenaming() - return - } - - // Set loading state - this.$set(this.source, 'status', NodeStatus.LOADING) - // Update node - this.source.rename(newName) - - logger.debug('Moving file to', { destination: this.source.encodedSource, oldEncodedSource }) try { - await axios({ - method: 'MOVE', - url: oldEncodedSource, - headers: { - Destination: this.source.encodedSource, - Overwrite: 'F', - }, - }) - - // Success 🎉 - emit('files:node:updated', this.source) - emit('files:node:renamed', this.source) - emit('files:node:moved', { - node: this.source, - oldSource: `${dirname(this.source.source)}/${oldName}`, - }) - showSuccess(t('files', 'Renamed "{oldName}" to "{newName}"', { oldName, newName })) - - // Reset the renaming store - this.stopRenaming() - this.$nextTick(() => { - const nameContainter = this.$refs.basename as HTMLElement | undefined - nameContainter?.focus() - }) + const status = await this.renamingStore.rename() + if (status) { + showSuccess(t('files', 'Renamed "{oldName}" to "{newName}"', { oldName, newName })) + this.$nextTick(() => { + const nameContainter = this.$refs.basename as HTMLElement | undefined + nameContainter?.focus() + }) + } else { + // Was cancelled - meaning the renaming state is just reset + } } catch (error) { - logger.error('Error while renaming file', { error }) - // Rename back as it failed - this.source.rename(oldName) + logger.error(error as Error) + showError((error as Error).message) // And ensure we reset to the renaming state this.startRenaming() - - if (isAxiosError(error)) { - // TODO: 409 means current folder does not exist, redirect ? - if (error?.response?.status === 404) { - showError(t('files', 'Could not rename "{oldName}", it does not exist any more', { oldName })) - return - } else if (error?.response?.status === 412) { - showError(t('files', 'The name "{newName}" is already used in the folder "{dir}". Please choose a different name.', { newName, dir: this.directory })) - return - } - } - - // Unknown error - showError(t('files', 'Could not rename "{oldName}"', { oldName })) - } finally { - this.$set(this.source, 'status', undefined) } }, diff --git a/apps/files/src/store/renaming.ts b/apps/files/src/store/renaming.ts index 3782b75e3a4..ff8ac3ba8da 100644 --- a/apps/files/src/store/renaming.ts +++ b/apps/files/src/store/renaming.ts @@ -2,17 +2,96 @@ * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { defineStore } from 'pinia' -import { subscribe } from '@nextcloud/event-bus' import type { Node } from '@nextcloud/files' import type { RenamingStore } from '../types' +import axios, { isAxiosError } from '@nextcloud/axios' +import { emit, subscribe } from '@nextcloud/event-bus' +import { NodeStatus } from '@nextcloud/files' +import { t } from '@nextcloud/l10n' +import { basename, dirname } from 'path' +import { defineStore } from 'pinia' +import logger from '../logger' +import Vue from 'vue' + export const useRenamingStore = function(...args) { const store = defineStore('renaming', { state: () => ({ renamingNode: undefined, newName: '', } as RenamingStore), + + actions: { + /** + * Execute the renaming. + * This will rename the node set as `renamingNode` to the configured new name `newName`. + * @return true if success, false if skipped (e.g. new and old name are the same) + * @throws Error if renaming fails, details are set in the error message + */ + async rename(): Promise<boolean> { + if (this.renamingNode === undefined) { + throw new Error('No node is currently being renamed') + } + + const newName = this.newName.trim?.() || '' + const oldName = this.renamingNode.basename + const oldEncodedSource = this.renamingNode.encodedSource + if (oldName === newName) { + return false + } + + const node = this.renamingNode + Vue.set(node, 'status', NodeStatus.LOADING) + + try { + // rename the node + this.renamingNode.rename(newName) + logger.debug('Moving file to', { destination: this.renamingNode.encodedSource, oldEncodedSource }) + // create MOVE request + await axios({ + method: 'MOVE', + url: oldEncodedSource, + headers: { + Destination: this.renamingNode.encodedSource, + Overwrite: 'F', + }, + }) + + // Success 🎉 + emit('files:node:updated', this.renamingNode as Node) + emit('files:node:renamed', this.renamingNode as Node) + emit('files:node:moved', { + node: this.renamingNode as Node, + oldSource: `${dirname(this.renamingNode.source)}/${oldName}`, + }) + this.$reset() + return true + } catch (error) { + logger.error('Error while renaming file', { error }) + // Rename back as it failed + this.renamingNode.rename(oldName) + if (isAxiosError(error)) { + // TODO: 409 means current folder does not exist, redirect ? + if (error?.response?.status === 404) { + throw new Error(t('files', 'Could not rename "{oldName}", it does not exist any more', { oldName })) + } else if (error?.response?.status === 412) { + throw new Error(t( + 'files', + 'The name "{newName}" is already used in the folder "{dir}". Please choose a different name.', + { + newName, + dir: basename(this.renamingNode.dirname), + }, + )) + } + } + // Unknown error + throw new Error(t('files', 'Could not rename "{oldName}"', { oldName })) + } finally { + Vue.set(node, 'status', undefined) + } + }, + }, }) const renamingStore = store(...args) diff --git a/cypress/e2e/files/files-renaming.cy.ts b/cypress/e2e/files/files-renaming.cy.ts index 9ac96a398b8..77f23a00459 100644 --- a/cypress/e2e/files/files-renaming.cy.ts +++ b/cypress/e2e/files/files-renaming.cy.ts @@ -4,7 +4,7 @@ */ import type { User } from '@nextcloud/cypress' -import { getRowForFile, haveValidity, triggerActionForFile } from './FilesUtils' +import { getRowForFile, haveValidity, renameFile, triggerActionForFile } from './FilesUtils' describe('files: Rename nodes', { testIsolation: true }, () => { let user: User @@ -115,4 +115,32 @@ describe('files: Rename nodes', { testIsolation: true }, () => { .findByRole('img', { name: 'File is loading' }) .should('not.exist') }) + + /** + * This is a regression test of: https://github.com/nextcloud/server/issues/47438 + * The issue was that the renaming state was not reset when the new name moved the file out of the view of the current files list + * due to virtual scrolling the renaming state was not changed then by the UI events (as the component was taken out of DOM before any event handling). + */ + it('correctly resets renaming state', () => { + for (let i = 1; i <= 20; i++) { + cy.uploadContent(user, new Blob([]), 'text/plain', `/file${i}.txt`) + } + cy.viewport(1200, 500) // 500px is smaller then 20 * 50 which is the place that the files take up + cy.login(user) + cy.visit('/apps/files') + + getRowForFile('file.txt').should('be.visible') + // Z so it is shown last + renameFile('file.txt', 'zzz.txt') + // not visible any longer + getRowForFile('zzz.txt').should('not.be.visible') + // scroll file list to bottom + cy.get('[data-cy-files-list]').scrollTo('bottom') + cy.screenshot() + // The file is no longer in rename state + getRowForFile('zzz.txt') + .should('be.visible') + .findByRole('textbox', { name: 'Filename' }) + .should('not.exist') + }) }) |