Skip to content

fix: Prevent moving a folder into itself - #2054

Draft
aymericmariaux wants to merge 14 commits into
mainfrom
move-folder-into-itself
Draft

fix: Prevent moving a folder into itself#2054
aymericmariaux wants to merge 14 commits into
mainfrom
move-folder-into-itself

Conversation

@aymericmariaux

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings July 17, 2026 11:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents folder moves into selected folders or their descendants by disabling invalid navigation destinations.

Changes:

  • Passes selected folder IDs into the destination picker.
  • Disables navigation into folders being moved.
  • Handles bulk “Select all” moves via the source parent ID.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
main_navigation.xml Adds destination-navigation arguments.
FileInfoActionsView.kt Supplies the current file to move validation.
Utils.kt Builds and passes disabled folder IDs.
GalleryFragment.kt Updates move argument naming.
SelectFolderFragment.kt Configures picker navigation restrictions.
SelectFolderActivity.kt Propagates restrictions through its ViewModel.
MultiSelectFragment.kt Supplies bulk-selection restrictions.
FileAdapter.kt Disables invalid destination folders.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

@aymericmariaux
aymericmariaux marked this pull request as draft July 21, 2026 12:32
@aymericmariaux
aymericmariaux force-pushed the move-folder-into-itself branch from b67e947 to dddae2a Compare July 30, 2026 13:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

app/src/main/java/com/infomaniak/drive/ui/fileList/FavoritesFragment.kt:75

  • These adapter restrictions only disable a moved folder itself (or selected direct children in select-all mode). Favorites is a flat list, so a favorite descendant of a moved folder does not match either condition; it remains clickable, openFavoriteFolder() navigates into it, and setupSaveButton() enables it because it only checks the original parent ID. Apply the ancestry check used for recent folders to favorite entries as well so descendants cannot be selected as destinations.
            disabledNavigationFolderIds = selectFolderViewModel.disabledNavigationFolderIds
            disabledNavigationParentFolderId = selectFolderViewModel.disabledNavigationParentFolderId
            exceptedNavigationFolderIds = selectFolderViewModel.exceptedNavigationFolderIds

app/src/main/java/com/infomaniak/drive/ui/MainViewModel.kt:351

  • translateError() maps limit_exceeded_error to the generic “Limit exceeded” resource, but move operations previously special-cased this response as “Limit of files in folder reached.” Because all three move result UIs now consume this errorResId, a full destination folder will show a less actionable message. Preserve the existing move-specific mapping before falling back to translateError().
                errorResId = apiResponse.translateError(defaultMessage = R.string.errorMove).takeIf { !apiResponse.isSuccess() },

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

app/src/main/java/com/infomaniak/drive/ui/MainViewModel.kt:252

  • This drops the previous limit-exceeded handling for individual operations whose FileResult still sets only errorCode; for example, duplicateFile() does so at line 494. Since the result handler now reads only errorResId, an individual copy that fails with limit_exceeded_error displays the generic error instead of errorFilesLimitExceeded. Preserve the error-code fallback here (or populate errorResId in every producer).
            errorResId = currentErrorResId ?: fileRequest.errorResId.takeIf { !fileRequest.isSuccess },

Comment thread app/src/main/java/com/infomaniak/drive/ui/fileList/FileAdapter.kt

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

Suppressed comments (2)

app/src/main/java/com/infomaniak/drive/ui/fileList/FileAdapter.kt:447

  • This only rejects the moved folder itself (or, for “select all,” an immediate child of the source parent). The Favorites and My Shares selectors can list folders from arbitrary depths, so a favorite/shared grandchild of a moved folder remains enabled; opening it also enables Save because setupSaveButton() only compares the destination ID. That still allows attempting to move a folder into its own subtree. Apply the ancestor-chain check used for recent folders to every shortcut-backed list, and validate the current destination before enabling Save as a final guard.
    private fun File.isNavigableFolder(): Boolean {
        val isMovedChildOfSource = parentId == disabledNavigationParentFolderId
                && id !in exceptedNavigationFolderIds
        return isFolder() && id !in disabledNavigationFolderIds && !isMovedChildOfSource

app/src/main/java/com/infomaniak/drive/ui/MainViewModel.kt:252

  • This drops the existing limit-exceeded message for multi-select copies. duplicateFile() still emits only errorCode, so a limit_exceeded_error now leaves errorResId null and handleIndividualActionsResult() shows the generic error instead of errorFilesLimitExceeded. Preserve the error-code fallback here (or populate errorResId for every producer) before removing errorCode from the mediator state.
            errorResId = currentErrorResId ?: fileRequest.errorResId.takeIf { !fileRequest.isSuccess },

@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.

Suppressed comments (2)

app/src/main/java/com/infomaniak/drive/ui/fileList/FileAdapter.kt:447

  • This only rejects the moved folder itself (or a direct child selected via “select all”), not deeper descendants. Flattened destinations such as Favorites and My Shares can display a descendant directly; for a move of A, favorite folder A/B passes these checks, can be opened, and the save button then allows moving A into B. Make this check ancestry-aware (or pass a precomputed set/predicate covering all forbidden descendants), as is attempted for recent folders.
    private fun File.isNavigableFolder(): Boolean {
        val isMovedChildOfSource = parentId == disabledNavigationParentFolderId
                && id !in exceptedNavigationFolderIds
        return isFolder() && id !in disabledNavigationFolderIds && !isMovedChildOfSource

app/src/main/java/com/infomaniak/drive/ui/fileList/SelectRootFolderFragment.kt:185

  • getParentFileProxy() follows the first Realm backlink, not necessarily the file's real parent. Files may also be linked from virtual containers such as Favorites/MyShares, so traversal can jump to that container and miss a moved ancestor, leaving a recent descendant enabled. Traverse the authoritative parentId instead.
                current = FileController.getParentFileProxy(current.id, realm = realm)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants