diff --git a/apps/files/lib/Service/UserConfig.php b/apps/files/lib/Service/UserConfig.php index 0abcfb2a6ad..b01caf9f960 100644 --- a/apps/files/lib/Service/UserConfig.php +++ b/apps/files/lib/Service/UserConfig.php @@ -18,6 +18,12 @@ class UserConfig { 'default' => true, 'allowed' => [true, false], ], + [ + // Whether to show the "confirm file extension change" warning + 'key' => 'show_dialog_file_extension', + 'default' => true, + 'allowed' => [true, false], + ], [ // Whether to show the hidden files or not in the files list 'key' => 'show_hidden', diff --git a/apps/files/src/components/FileEntry/FileEntryName.vue b/apps/files/src/components/FileEntry/FileEntryName.vue index bc6b61ad54c..2fec9e5d556 100644 --- a/apps/files/src/components/FileEntry/FileEntryName.vue +++ b/apps/files/src/components/FileEntry/FileEntryName.vue @@ -23,7 +23,6 @@ @@ -117,11 +116,11 @@ export default defineComponent({ return this.isRenaming && this.filesListWidth < 512 }, newName: { - get() { - return this.renamingStore.newName + get(): string { + return this.renamingStore.newNodeName }, - set(newName) { - this.renamingStore.newName = newName + set(newName: string) { + this.renamingStore.newNodeName = newName }, }, @@ -249,7 +248,9 @@ export default defineComponent({ try { const status = await this.renamingStore.rename() if (status) { - showSuccess(t('files', 'Renamed "{oldName}" to "{newName}"', { oldName, newName })) + showSuccess( + t('files', 'Renamed "{oldName}" to "{newName}"', { oldName, newName: this.source.basename }), + ) this.$nextTick(() => { const nameContainer = this.$refs.basename as HTMLElement | undefined nameContainer?.focus() diff --git a/apps/files/src/eventbus.d.ts b/apps/files/src/eventbus.d.ts index ce0611580fe..49ac67fe4db 100644 --- a/apps/files/src/eventbus.d.ts +++ b/apps/files/src/eventbus.d.ts @@ -16,6 +16,7 @@ declare module '@nextcloud/event-bus' { 'files:node:created': Node 'files:node:deleted': Node 'files:node:updated': Node + 'files:node:rename': Node 'files:node:renamed': Node 'files:node:moved': { node: Node, oldSource: string } diff --git a/apps/files/src/store/renaming.ts b/apps/files/src/store/renaming.ts index 8ed455a182a..2ac9e06ba16 100644 --- a/apps/files/src/store/renaming.ts +++ b/apps/files/src/store/renaming.ts @@ -3,184 +3,165 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ 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 { FileType, NodeStatus } from '@nextcloud/files' -import { DialogBuilder } from '@nextcloud/dialogs' import { t } from '@nextcloud/l10n' +import { spawnDialog } from '@nextcloud/vue/functions/dialog' import { basename, dirname, extname } from 'path' import { defineStore } from 'pinia' import logger from '../logger' -import Vue from 'vue' -import IconCancel from '@mdi/svg/svg/cancel.svg?raw' -import IconCheck from '@mdi/svg/svg/check.svg?raw' +import Vue, { defineAsyncComponent, ref } from 'vue' +import { useUserConfigStore } from './userconfig' -let isDialogVisible = false +export const useRenamingStore = defineStore('renaming', () => { + /** + * The currently renamed node + */ + const renamingNode = ref() + /** + * The new name of the currently renamed node + */ + const newNodeName = ref('') -const showWarningDialog = (oldExtension: string, newExtension: string): Promise => { - if (isDialogVisible) { - return Promise.resolve(false) - } + /** + * Internal flag to only allow calling `rename` once. + */ + const isRenaming = ref(false) - isDialogVisible = true + /** + * 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 function rename(): Promise { + if (renamingNode.value === undefined) { + throw new Error('No node is currently being renamed') + } - let message + // Only rename once so we use this as some kind of mutex + if (isRenaming.value) { + return false + } + isRenaming.value = true - if (!oldExtension && newExtension) { - message = t( - 'files', - 'Adding the file extension "{new}" may render the file unreadable.', - { new: newExtension }, - ) - } else if (!newExtension) { - message = t( - 'files', - 'Removing the file extension "{old}" may render the file unreadable.', - { old: oldExtension }, - ) - } else { - message = t( - 'files', - 'Changing the file extension from "{old}" to "{new}" may render the file unreadable.', - { old: oldExtension, new: newExtension }, - ) - } + const node = renamingNode.value + Vue.set(node, 'status', NodeStatus.LOADING) - return new Promise((resolve) => { - const dialog = new DialogBuilder() - .setName(t('files', 'Change file extension')) - .setText(message) - .setButtons([ - { - label: t('files', 'Keep {oldextension}', { oldextension: oldExtension }), - icon: IconCancel, - type: 'secondary', - callback: () => { - isDialogVisible = false - resolve(false) - }, + const userConfig = useUserConfigStore() + + let newName = newNodeName.value.trim() + const oldName = node.basename + const oldExtension = extname(oldName) + const newExtension = extname(newName) + // Check for extension change for files + if (node.type === FileType.File + && oldExtension !== newExtension + && userConfig.userConfig.show_dialog_file_extension + && !(await showFileExtensionDialog(oldExtension, newExtension)) + ) { + // user selected to use the old extension + newName = basename(newName, newExtension) + oldExtension + } + + const oldEncodedSource = node.encodedSource + try { + if (oldName === newName) { + return false + } + + // rename the node + node.rename(newName) + logger.debug('Moving file to', { destination: node.encodedSource, oldEncodedSource }) + // create MOVE request + await axios({ + method: 'MOVE', + url: oldEncodedSource, + headers: { + Destination: node.encodedSource, + Overwrite: 'F', }, - { - label: newExtension.length ? t('files', 'Use {newextension}', { newextension: newExtension }) : t('files', 'Remove extension'), - icon: IconCheck, - type: 'primary', - callback: () => { - isDialogVisible = false - resolve(true) - }, - }, - ]) - .build() + }) - dialog.show().then(() => { - dialog.hide() - }) - }) -} + // Success 🎉 + emit('files:node:updated', node) + emit('files:node:renamed', node) + emit('files:node:moved', { + node, + oldSource: `${dirname(node.source)}/${oldName}`, + }) -export const useRenamingStore = function(...args) { - const store = defineStore('renaming', { - state: () => ({ - renamingNode: undefined, - newName: '', - } as RenamingStore), + // Reset the state not changed + if (renamingNode.value === node) { + $reset() + } - 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 { - 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 - - // Check for extension change for files - const oldExtension = extname(oldName) - const newExtension = extname(newName) - if (oldExtension !== newExtension && this.renamingNode.type === FileType.File) { - const proceed = await showWarningDialog(oldExtension, newExtension) - if (!proceed) { - return false - } - } - - 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', + return true + } catch (error) { + logger.error('Error while renaming file', { error }) + // Rename back as it failed + node.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(renamingNode.value!.dirname), }, - }) - - // 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) + )) } - }, - }, - }) + } + // Unknown error + throw new Error(t('files', 'Could not rename "{oldName}"', { oldName })) + } finally { + Vue.set(node, 'status', undefined) + isRenaming.value = false + } + } - const renamingStore = store(...args) + /** + * Reset the store state + */ + function $reset(): void { + newNodeName.value = '' + renamingNode.value = undefined + } // Make sure we only register the listeners once - if (!renamingStore._initialized) { - subscribe('files:node:rename', function(node: Node) { - renamingStore.renamingNode = node - renamingStore.newName = node.basename - }) - renamingStore._initialized = true - } + subscribe('files:node:rename', (node: Node) => { + renamingNode.value = node + newNodeName.value = node.basename + }) - return renamingStore + return { + $reset, + + newNodeName, + rename, + renamingNode, + } +}) + +/** + * Show a dialog asking user for confirmation about changing the file extension. + * + * @param oldExtension the old file name extension + * @param newExtension the new file name extension + */ +async function showFileExtensionDialog(oldExtension: string, newExtension: string): Promise { + const { promise, resolve } = Promise.withResolvers() + spawnDialog( + defineAsyncComponent(() => import('../views/DialogConfirmFileExtension.vue')), + { oldExtension, newExtension }, + (useNewExtension: unknown) => resolve(Boolean(useNewExtension)), + ) + return await promise } diff --git a/apps/files/src/store/userconfig.ts b/apps/files/src/store/userconfig.ts index 95829c849e8..d84a5e0d935 100644 --- a/apps/files/src/store/userconfig.ts +++ b/apps/files/src/store/userconfig.ts @@ -17,6 +17,8 @@ const initialUserConfig = loadState('files', 'config', { sort_favorites_first: true, sort_folders_first: true, grid_view: false, + + show_dialog_file_extension: true, }) export const useUserConfigStore = defineStore('userconfig', () => { diff --git a/apps/files/src/types.ts b/apps/files/src/types.ts index 62ba53b89d2..4bf8a557f49 100644 --- a/apps/files/src/types.ts +++ b/apps/files/src/types.ts @@ -52,6 +52,7 @@ export interface PathOptions { export interface UserConfig { [key: string]: boolean|undefined + show_dialog_file_extension: boolean, show_hidden: boolean crop_image_previews: boolean sort_favorites_first: boolean diff --git a/apps/files/src/views/DialogConfirmFileExtension.cy.ts b/apps/files/src/views/DialogConfirmFileExtension.cy.ts new file mode 100644 index 00000000000..460497dd91f --- /dev/null +++ b/apps/files/src/views/DialogConfirmFileExtension.cy.ts @@ -0,0 +1,161 @@ +/*! + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +import { createTestingPinia } from '@pinia/testing' +import DialogConfirmFileExtension from './DialogConfirmFileExtension.vue' +import { useUserConfigStore } from '../store/userconfig' + +describe('DialogConfirmFileExtension', () => { + it('renders with both extensions', () => { + cy.mount(DialogConfirmFileExtension, { + propsData: { + oldExtension: '.old', + newExtension: '.new', + }, + global: { + plugins: [createTestingPinia({ + createSpy: cy.spy, + })], + }, + }) + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('heading') + .should('contain.text', 'Change file extension') + cy.get('@dialog') + .findByRole('checkbox', { name: /Do not show this dialog again/i }) + .should('exist') + .and('not.be.checked') + cy.get('@dialog') + .findByRole('button', { name: 'Keep .old' }) + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Use .new' }) + .should('be.visible') + }) + + it('renders without old extension', () => { + cy.mount(DialogConfirmFileExtension, { + propsData: { + newExtension: '.new', + }, + global: { + plugins: [createTestingPinia({ + createSpy: cy.spy, + })], + }, + }) + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Keep without extension' }) + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Use .new' }) + .should('be.visible') + }) + + it('renders without new extension', () => { + cy.mount(DialogConfirmFileExtension, { + propsData: { + oldExtension: '.old', + }, + global: { + plugins: [createTestingPinia({ + createSpy: cy.spy, + })], + }, + }) + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Keep .old' }) + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Remove extension' }) + .should('be.visible') + }) + + it('emits correct value on keep old', () => { + cy.mount(DialogConfirmFileExtension, { + propsData: { + oldExtension: '.old', + newExtension: '.new', + }, + global: { + plugins: [createTestingPinia({ + createSpy: cy.spy, + })], + }, + }).as('component') + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Keep .old' }) + .click() + cy.get('@component') + .its('wrapper') + .should((wrapper) => expect(wrapper.emitted('close')).to.eql([[false]])) + }) + + it('emits correct value on use new', () => { + cy.mount(DialogConfirmFileExtension, { + propsData: { + oldExtension: '.old', + newExtension: '.new', + }, + global: { + plugins: [createTestingPinia({ + createSpy: cy.spy, + })], + }, + }).as('component') + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('button', { name: 'Use .new' }) + .click() + cy.get('@component') + .its('wrapper') + .should((wrapper) => expect(wrapper.emitted('close')).to.eql([[true]])) + }) + + it('updates user config when checking the checkbox', () => { + const pinia = createTestingPinia({ + createSpy: cy.spy, + }) + + cy.mount(DialogConfirmFileExtension, { + propsData: { + oldExtension: '.old', + newExtension: '.new', + }, + global: { + plugins: [pinia], + }, + }).as('component') + + cy.findByRole('dialog') + .as('dialog') + .should('be.visible') + cy.get('@dialog') + .findByRole('checkbox', { name: /Do not show this dialog again/i }) + .check({ force: true }) + + cy.wrap(useUserConfigStore()) + .its('update') + .should('have.been.calledWith', 'show_dialog_file_extension', false) + }) +}) diff --git a/apps/files/src/views/DialogConfirmFileExtension.vue b/apps/files/src/views/DialogConfirmFileExtension.vue new file mode 100644 index 00000000000..cc1ee363f98 --- /dev/null +++ b/apps/files/src/views/DialogConfirmFileExtension.vue @@ -0,0 +1,92 @@ + + + + + + + diff --git a/apps/files/src/views/Settings.vue b/apps/files/src/views/Settings.vue index 1def0e41c15..872b8a8e6d3 100644 --- a/apps/files/src/views/Settings.vue +++ b/apps/files/src/views/Settings.vue @@ -83,6 +83,15 @@ + + {{ t('files', 'Prevent warning dialogs from open or reenable them.') }} + + {{ t('files', 'Show a warning dialog when changing a file extension.') }} + + + {{ t('files', 'Speed up your Files experience with these quick shortcuts.') }} diff --git a/cypress/e2e/files/files-renaming.cy.ts b/cypress/e2e/files/files-renaming.cy.ts index 0e698432102..269f0bd252b 100644 --- a/cypress/e2e/files/files-renaming.cy.ts +++ b/cypress/e2e/files/files-renaming.cy.ts @@ -74,7 +74,7 @@ describe('files: Rename nodes', { testIsolation: true }, () => { }) it('shows accessible loading information', () => { - const { resolve, promise } = Promise.withResolvers() + const { resolve, promise } = Promise.withResolvers() getRowForFile('file.txt').should('be.visible') @@ -106,7 +106,7 @@ describe('files: Rename nodes', { testIsolation: true }, () => { .should('not.exist') cy.log('Resolve promise to preoceed with MOVE request') - .then(() => resolve(null)) + .then(() => resolve()) // Ensure the request is done (file renamed) cy.wait('@moveFile') @@ -194,7 +194,7 @@ describe('files: Rename nodes', { testIsolation: true }, () => { .should('not.exist') }) - it('shows warning on extension change', () => { + it('shows warning on extension change - select new extension', () => { getRowForFile('file.txt').should('be.visible') triggerActionForFile('file.txt', 'rename') @@ -202,39 +202,60 @@ describe('files: Rename nodes', { testIsolation: true }, () => { .findByRole('textbox', { name: 'Filename' }) .should('be.visible') .type('{selectAll}file.md') - .should(haveValidity('')) .type('{enter}') // See warning dialog cy.findByRole('dialog', { name: 'Change file extension' }) .should('be.visible') - .findByRole('button', { name: /use/i }) + .findByRole('button', { name: 'Use .md' }) .click() // See it is renamed getRowForFile('file.md').should('be.visible') }) - it('shows warning on extension change and allow cancellation', () => { + it('shows warning on extension change - select old extension', () => { getRowForFile('file.txt').should('be.visible') triggerActionForFile('file.txt', 'rename') getRowForFile('file.txt') .findByRole('textbox', { name: 'Filename' }) .should('be.visible') - .type('{selectAll}file.md') - .should(haveValidity('')) + .type('{selectAll}document.md') .type('{enter}') // See warning dialog cy.findByRole('dialog', { name: 'Change file extension' }) .should('be.visible') - .findByRole('button', { name: /keep/i }) + .findByRole('button', { name: 'Keep .txt' }) .click() - // See it is not renamed + // See it is renamed + getRowForFile('document.txt').should('be.visible') + }) + + it('shows warning on extension removal', () => { getRowForFile('file.txt').should('be.visible') - getRowForFile('file.md').should('not.exist') + + triggerActionForFile('file.txt', 'rename') + getRowForFile('file.txt') + .findByRole('textbox', { name: 'Filename' }) + .should('be.visible') + .type('{selectAll}file') + .type('{enter}') + + cy.findByRole('dialog', { name: 'Change file extension' }) + .should('be.visible') + .findByRole('button', { name: 'Keep .txt' }) + .should('be.visible') + cy.findByRole('dialog', { name: 'Change file extension' }) + .findByRole('button', { name: 'Remove extension' }) + .should('be.visible') + .click() + + // See it is renamed + getRowForFile('file').should('be.visible') + getRowForFile('file.txt').should('not.exist') }) it('does not show warning on folder renaming with a dot', () => {