fix(MoveMailboxModal): make self-move a real no-op and always reset state - #13661
Open
chiliec wants to merge 1 commit into
Open
fix(MoveMailboxModal): make self-move a real no-op and always reset state#13661chiliec wants to merge 1 commit into
chiliec wants to merge 1 commit into
Conversation
…tate 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 <vovababin@gmail.com>
chiliec
requested review from
ChristophWurst,
GretaD and
kesselb
as code owners
September 9, 2026 13:08
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Closes #13534
Fixes the dead self-move guard in the move-folder modal.
The bug
onMove()setthis.moving = trueand then guarded the whole operation withthis.mailbox.id !== this.destMailboxId:Mailbox::jsonSerializeexposesidasbase64_encode($name), whiledestMailboxIdis always the numericdatabaseId(orundefined). The two can never be equal, so the "moving a folder into itself is a no-op" check never fired — selecting the current parent for a folder that already sits there issued a rename to its own name, and the resulting IMAP error was only logged (silent no-op for the user).moving = false/$emit('close')cleanup lived inside thefinallyof that dead branch, so a corrected early exit would have left the dialog's primary button disabled forever (since fix: minor adjustment in moving messages, threads and folders #13431 made the handler's promise the oneNcDialogButtonawaits).Fix
this.mailbox.databaseIdagainstthis.destMailboxId.close) when they match, before settingmoving = true.try/finallynow covers the actual move, somovingis always reset and the dialog always closes.Tests
Added
src/tests/unit/components/MoveMailboxModal.vue.spec.jscovering: same-mailbox no-op (norenameMailbox,movingreset,closeemitted), a real move with correct new name, and state reset after a failed move.Validation
Genuine RED→GREEN: reverting
src/components/MoveMailboxModal.vueto the buggy version makes the "same mailbox is a no-op" test fail (the old code callsrenameMailboxwhen moving into itself); restoring the fix turns it green.npx eslinton both changed files is clean. I ran the fullvitest --runsuite too — a handful of unrelated files (TranslationModal, MessageService, store/actions, filePicker, languageDetection) fail non-deterministically with jsdom timeouts on the same count on a cleanmaincheckout without my change, so they are pre-existing and outside this diff.Happy to adjust.