Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Six unresolved moderate findings affect UI state, request cancellation, result completeness, and folder navigation.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds cloud-wide full-text file search via WebDAV REPORT, local fallback, and debounced file-list integration.
Changes:
- Adds remote/local search APIs, persistence, and dependency injection.
- Implements space-aware WebDAV result parsing and legacy fallback.
- Updates UI search behavior, paths, translations, and tests.
File summaries
| File | Reviewed change | Final review comments |
|---|---|---|
opencloudDomain/src/test/java/eu/opencloud/android/domain/files/usecases/SearchFilesUseCaseTest.kt |
Tests search use-case results and errors. | — |
opencloudDomain/src/main/java/eu/opencloud/android/domain/files/usecases/SearchFilesUseCase.kt |
Adds the search use case. | — |
opencloudDomain/src/main/java/eu/opencloud/android/domain/files/FileRepository.kt |
Defines the repository search contract. | — |
opencloudData/src/test/java/eu/opencloud/android/data/files/repository/OCFileRepositoryTest.kt |
Tests repository search mapping. | — |
opencloudData/src/test/java/eu/opencloud/android/data/files/datasources/implementation/OCRemoteFileDataSourceTest.kt |
Tests remote search delegation. | — |
opencloudData/src/test/java/eu/opencloud/android/data/files/datasources/implementation/OCLocalFileDataSourceTest.kt |
Tests local search and sync lookup. | — |
opencloudData/src/main/java/eu/opencloud/android/data/files/repository/OCFileRepository.kt |
Implements remote/local search integration and persistence. | Moderate (2 votes): Search-result folders can lack parent relationships, breaking browse-up navigation. |
opencloudData/src/main/java/eu/opencloud/android/data/files/db/FileDao.kt |
Adds account-wide local search. | — |
opencloudData/src/main/java/eu/opencloud/android/data/files/datasources/RemoteFileDataSource.kt |
Defines the remote search interface. | — |
opencloudData/src/main/java/eu/opencloud/android/data/files/datasources/LocalFileDataSource.kt |
Defines local search and sync interfaces. | — |
opencloudData/src/main/java/eu/opencloud/android/data/files/datasources/implementation/OCRemoteFileDataSource.kt |
Maps remote search results. | — |
opencloudData/src/main/java/eu/opencloud/android/data/files/datasources/implementation/OCLocalFileDataSource.kt |
Implements local search and sync lookup. | — |
opencloudComLibrary/src/test/java/eu/opencloud/android/lib/resources/files/SearchRemoteFilesOperationTest.kt |
Tests remote search and legacy fallback. | — |
opencloudComLibrary/src/test/java/eu/opencloud/android/lib/RemoteFileTest.kt |
Tests space-aware path parsing. | — |
opencloudComLibrary/src/test/java/eu/opencloud/android/lib/common/http/methods/webdav/SearchMethodTest.kt |
Tests REPORT XML generation and parsing. | — |
opencloudComLibrary/src/main/java/eu/opencloud/android/lib/resources/files/services/implementation/OCFileService.kt |
Implements the file-service search API. | — |
opencloudComLibrary/src/main/java/eu/opencloud/android/lib/resources/files/services/FileService.kt |
Exposes file search. | — |
opencloudComLibrary/src/main/java/eu/opencloud/android/lib/resources/files/SearchRemoteFilesOperation.kt |
Executes remote search and fallback. | Moderate (1 vote): A hard limit of 100 results silently omits additional matches. |
opencloudComLibrary/src/main/java/eu/opencloud/android/lib/resources/files/RemoteFile.kt |
Adds space metadata and search-result parsing. | — |
opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/http/methods/webdav/SearchMethod.kt |
Builds and executes search REPORT requests. | — |
opencloudComLibrary/src/main/java/eu/opencloud/android/lib/common/http/methods/webdav/DavReportResource.kt |
Adds REPORT request support. | — |
opencloudApp/src/test/java/eu/opencloud/android/presentation/viewmodels/KeyAppViewModelsTest.kt |
Tests ViewModel search integration. | — |
opencloudApp/src/main/res/values/strings.xml |
Updates the English search label. | — |
opencloudApp/src/main/res/values-pl/strings.xml |
Updates the Polish search label. | — |
opencloudApp/src/main/res/values-nl/strings.xml |
Updates the Dutch search label. | — |
opencloudApp/src/main/res/values-it/strings.xml |
Updates the Italian search label. | — |
opencloudApp/src/main/res/values-fr/strings.xml |
Updates the French search label. | — |
opencloudApp/src/main/res/values-es/strings.xml |
Updates the Spanish search label. | — |
opencloudApp/src/main/res/values-de/strings.xml |
Updates the German search label. | — |
opencloudApp/src/main/java/eu/opencloud/android/presentation/files/filelist/MainFileListViewModel.kt |
Adds debounced global search state and execution. | Moderate (1 vote): Search failures are converted into empty successful results. Moderate (2 votes): In-flight blocking requests are not cancelled. |
opencloudApp/src/main/java/eu/opencloud/android/presentation/files/filelist/MainFileListFragment.kt |
Connects search UI and result navigation. | — |
opencloudApp/src/main/java/eu/opencloud/android/presentation/files/filelist/FileListAdapter.kt |
Displays paths for search results. | Moderate (2 votes): Search-mode changes do not trigger row rebinding. Moderate (1 vote): Reused holders can retain stale space metadata. |
opencloudApp/src/main/java/eu/opencloud/android/dependecyinjection/ViewModelModule.kt |
Injects the search dependency. | — |
opencloudApp/src/main/java/eu/opencloud/android/dependecyinjection/UseCaseModule.kt |
Registers the search use case. | — |
Review details
Suppressed comments (3)
opencloudApp/src/main/java/eu/opencloud/android/presentation/files/filelist/FileListAdapter.kt:339
- Search mode makes this branch run for results whose
spaceis null, but theletbelow is skipped in that case. Because RecyclerView holders are reused, a row previously bound to a space result can retain its space icon and name while displaying a personal/legacy result. Reset those two views before applying the nullablespacedata.
isSearchActive
opencloudApp/src/main/java/eu/opencloud/android/presentation/files/filelist/MainFileListViewModel.kt:425
- When the use case returns an error,
getDataOrNull()is null and this expression converts the failure into an empty successful result. The fragment therefore displays the no-matches state for authentication, network, or database failures, hiding the actual search error; preserve the error in the UI state and handle it separately.
val filesWithSyncInfo = (searchResult.getDataOrNull() ?: emptyList())
opencloudComLibrary/src/main/java/eu/opencloud/android/lib/resources/files/SearchRemoteFilesOperation.kt:46
- This operation performs one request with a hard limit of 100 and has no pagination or continuation handling. A query matching more than 100 files silently omits the remaining matches, so the new full-cloud search does not return all searchable files; page through the report results or otherwise expose/handle truncation.
val limit: Int = SearchMethod.DEFAULT_SEARCH_LIMIT,
- Files reviewed: 34/34 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+109
to
+112
| @OptIn(FlowPreview::class) | ||
| private val debouncedSearchFilter: Flow<String> = searchFilter.debounce { query -> | ||
| if (query.isBlank()) 0L else SEARCH_DEBOUNCE_MS | ||
| } |
| localFileDataSource.saveFile(remoteFile) | ||
| remoteFile | ||
| } else { | ||
| localFileDataSource.saveFile(remoteFile) |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… entries
- Check remotePath == ROOT_PATH instead of parentId == ROOT_PARENT_ID
in FileDisplayActivity and FolderPickerActivity onBackPressed() to
prevent subfolders with null/0 parent IDs from exiting the app
- Resolve parent directory via getParentRemotePath() in
MainFileListViewModel.manageBrowseUp() when parentId is missing
instead of throwing NotImplementedError via TODO()
- Avoid using java.io.File path operations in OCFile for remote WebDAV
paths to ensure forward slashes across all platforms
- Prioritize valid parent IDs in FileDao.getFileByOwnerAndRemotePath
and reuse existing row IDs in insertFilesInFolder... to prevent and
clean up duplicate rows for the same remotePath
- Resolve parentId from parent remote path in OCFileRepository.searchFiles()
when saving newly discovered items
- Fix OCFileEntity.fromCursor() reading NULL parent IDs as 0L instead of null
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.
fixes #98
currently you can use query like "content:" to search for content inside files. by default only titles will be searched but now across your whole cloud not the current folder only.
later we might be able to also introduce a gui to be able to comfortably build such a query. you can already do this by hand by searching for "content:xxx AND .pdf" for example