From 42bffc0ecb33ec8ce56fb3b117ffa1235e828440 Mon Sep 17 00:00:00 2001 From: Vladimir Babin Date: Wed, 9 Sep 2026 16:07:49 +0300 Subject: [PATCH] fix(MoveMailboxModal): make self-move a real no-op and always reset state The self-move guard compared the base64-encoded mailbox id against the numeric destMailboxId, so it never matched and a folder moved onto itself issued a rename to its own name. The state cleanup also lived inside that dead branch, so an early return would have left the dialog stuck. Compare databaseId, return early on a match, and hoist the finally cleanup so moving is always reset and the dialog closes. Assisted-by: Claude Code:claude-opus-4-8 Signed-off-by: Vladimir Babin --- CHANGELOG.md | 1 + src/components/MoveMailboxModal.vue | 51 +++++++------ .../components/MoveMailboxModal.vue.spec.js | 73 +++++++++++++++++++ 3 files changed, 101 insertions(+), 24 deletions(-) create mode 100644 src/tests/unit/components/MoveMailboxModal.vue.spec.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 51a5bcb052..5d3f0b9617 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ All notable changes to this project will be documented in this file. * Prefix the internal-address index with the user id ### Fixed * Quick actions for special folders +* Prevent a stuck dialog when moving a folder onto itself * Show the unified inbox icon * Detect message language in the browser for translation * Skip IMAP fetch results without a UID during sync diff --git a/src/components/MoveMailboxModal.vue b/src/components/MoveMailboxModal.vue index 530dfdaddc..c6d3baa351 100644 --- a/src/components/MoveMailboxModal.vue +++ b/src/components/MoveMailboxModal.vue @@ -56,33 +56,36 @@ export default { }, async onMove() { + if (this.mailbox.databaseId === this.destMailboxId) { + this.$emit('close') + return + } + this.moving = true - if (this.mailbox.id !== this.destMailboxId) { - try { - if (!this.destMailboxId) { - const newName = this.mailbox.displayName - await this.mainStore.renameMailbox({ - account: this.account, - mailbox: this.mailbox, - newName, - }) - } else { - const destMailbox = this.mainStore.getMailbox(this.destMailboxId) - const newName = destMailbox.name + this.mailbox.delimiter + this.mailbox.displayName - await this.mainStore.renameMailbox({ - account: this.account, - mailbox: this.mailbox, - newName, - }) - } - } catch (error) { - logger.error('could not move folder', { - error, + try { + if (!this.destMailboxId) { + const newName = this.mailbox.displayName + await this.mainStore.renameMailbox({ + account: this.account, + mailbox: this.mailbox, + newName, + }) + } else { + const destMailbox = this.mainStore.getMailbox(this.destMailboxId) + const newName = destMailbox.name + this.mailbox.delimiter + this.mailbox.displayName + await this.mainStore.renameMailbox({ + account: this.account, + mailbox: this.mailbox, + newName, }) - } finally { - this.moving = false - this.$emit('close') } + } catch (error) { + logger.error('could not move folder', { + error, + }) + } finally { + this.moving = false + this.$emit('close') } }, diff --git a/src/tests/unit/components/MoveMailboxModal.vue.spec.js b/src/tests/unit/components/MoveMailboxModal.vue.spec.js new file mode 100644 index 0000000000..e31276ca5c --- /dev/null +++ b/src/tests/unit/components/MoveMailboxModal.vue.spec.js @@ -0,0 +1,73 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { createTestingPinia } from '@pinia/testing' +import { createLocalVue, shallowMount } from '@vue/test-utils' +import { PiniaVuePlugin, setActivePinia } from 'pinia' +import MoveMailboxModal from '../../../components/MoveMailboxModal.vue' +import Nextcloud from '../../../mixins/Nextcloud.js' +import useMainStore from '../../../store/mainStore.js' + +const localVue = createLocalVue() +localVue.use(PiniaVuePlugin) +localVue.mixin(Nextcloud) + +describe('MoveMailboxModal', () => { + let store + + const mount = (mailbox) => shallowMount(MoveMailboxModal, { + propsData: { + account: { accountId: 1 }, + mailbox, + }, + localVue, + }) + + beforeEach(() => { + setActivePinia(createTestingPinia()) + store = useMainStore() + store.renameMailbox = vi.fn().mockResolvedValue() + }) + + it('treats selecting the same mailbox as a no-op and closes', async () => { + store.getMailbox = vi.fn().mockReturnValue({ name: 'INBOX' }) + const view = mount({ databaseId: 7, id: 'SU5CT1g=', displayName: 'INBOX', delimiter: '.' }) + view.vm.destMailboxId = 7 + + await view.vm.onMove() + + expect(store.renameMailbox).not.toHaveBeenCalled() + expect(view.vm.moving).toBe(false) + expect(view.emitted().close).toBeTruthy() + }) + + it('moves the folder when the destination differs and resets state', async () => { + store.getMailbox = vi.fn().mockReturnValue({ name: 'Archive' }) + const view = mount({ databaseId: 7, id: 'SU5CT1g=', displayName: 'INBOX', delimiter: '.' }) + view.vm.destMailboxId = 9 + + await view.vm.onMove() + + expect(store.renameMailbox).toHaveBeenCalledWith({ + account: { accountId: 1 }, + mailbox: view.vm.mailbox, + newName: 'Archive.INBOX', + }) + expect(view.vm.moving).toBe(false) + expect(view.emitted().close).toBeTruthy() + }) + + it('resets moving state after a failed move', async () => { + store.getMailbox = vi.fn().mockReturnValue({ name: 'Archive' }) + store.renameMailbox = vi.fn().mockRejectedValue(new Error('IMAP error')) + const view = mount({ databaseId: 7, id: 'SU5CT1g=', displayName: 'INBOX', delimiter: '.' }) + view.vm.destMailboxId = 9 + + await view.vm.onMove() + + expect(view.vm.moving).toBe(false) + expect(view.emitted().close).toBeTruthy() + }) +})