fix: order and limit the DAV search so "Recent" is actually recent - #100
Open
karlitschek wants to merge 1 commit into
Open
fix: order and limit the DAV search so "Recent" is actually recent#100karlitschek wants to merge 1 commit into
karlitschek wants to merge 1 commit into
Conversation
The office file search sent neither <d:orderby> nor <d:limit>. Without an
explicit limit the server falls back to its own default of 100 rows
(FileSearchBackend::transformQuery()), and without an ordering the query
runs with no ORDER BY at all — so the client received an arbitrary
100-row slice in database order and then sorted *that* by mtime.
For anyone with more than ~100 office files this means "Recent" could
omit the document they edited minutes ago, silently: the list still looks
plausible, it is just not the newest files. It also made MAX_DISPLAY_FILES
(200) and the "Show all in Files" affordance unreachable, since the result
set could never exceed 100.
Both knobs are supported by the DAV search grammar already, so this is a
request-side fix:
- order by {DAV:}getlastmodified descending, which the backend maps to the
mtime column and pushes down into the query
- request SEARCH_RESULT_LIMIT (500) rows explicitly, comfortably above
MAX_DISPLAY_FILES so a single category is not starved by the others
after category and Mine/Shared filtering
getAllOfficeFiles() now also reports whether the limit truncated the
result set, and the "Show all in Files" button appears in that case too,
rather than implying the page shows everything.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR updates the WebDAV SEARCH request used by the Office overview so that “Recent” actually reflects newest files and the UI can indicate when the server-side result set was capped.
Changes:
- Add explicit DAV
<d:orderby>(mtime desc) and<d:limit>(500) to the office mime SEARCH request. - Change
getAllOfficeFiles()to return{ nodes, truncated }and bubble truncation into the overview to enable the “Show all in Files” escape hatch. - Update bundled CSS import to a new chunk name and adjust the chunk content accordingly.
Reviewed changes
Copilot reviewed 3 out of 6 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/views/OfficeOverview.vue | Tracks server truncation and uses it to decide when “Show all in Files” should appear. |
| src/services/officeFiles.ts | Adds DAV order/limit and returns richer search results including a truncation flag. |
| css/office-main.css | Points the app CSS entry to a new generated chunk filename. |
| css/main-DBa1IQMb.chunk.css | Updates generated scoped-style content (hash changes). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,2 +1,2 @@ | |||
| /* extracted by css-entry-points-plugin */ | |||
| @import './main-DBa1IQMb.chunk.css'; No newline at end of file | |||
| @import './main-6D1uwA8_.chunk.css'; No newline at end of file | |||
Comment on lines
+93
to
+100
| cachedResult = { | ||
| nodes: results | ||
| .map(item => resultToNode(item as Parameters<typeof resultToNode>[0])) | ||
| .filter(node => node.type === 'file'), | ||
| // A full page back means the limit, not the collection, ended the result set. | ||
| // Measured on the raw rows: the folder filter below can only shrink the count. | ||
| truncated: results.length >= SEARCH_RESULT_LIMIT, | ||
| } |
| .filter(node => node.type === 'file'), | ||
| // A full page back means the limit, not the collection, ended the result set. | ||
| // Measured on the raw rows: the folder filter below can only shrink the count. | ||
| truncated: results.length >= SEARCH_RESULT_LIMIT, |
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.
The problem
buildOfficeMimeSearch()sent neither<d:orderby>nor<d:limit>.<d:limit>, the server applies its own default of 100 rows (FileSearchBackend::transformQuery()).<d:orderby>, the query is built with noORDER BYat all.So the client received an arbitrary 100-row slice in database order (roughly fileid/insertion order) and then sorted that slice by mtime before labelling it "Recent Documents".
For any user with more than ~100 office files, the document they edited this morning may simply not be on the page — and it fails silently, because the list still looks plausible.
Knock-on:
MAX_DISPLAY_FILES(200) and thehasMoreFiles→ "Show all in Files" affordance were unreachable, since the result set could never exceed 100.The fix
Both knobs are already supported by the DAV search grammar —
searchdavparses{DAV:}orderbyand{DAV:}limit, andFileSearchBackendpushes both down into the DB query — so this is purely request-side:{DAV:}getlastmodifieddescending, which the backend maps to themtimecolumn. This is what makes the result set the newest files rather than an arbitrary one.SEARCH_RESULT_LIMIT(500) rows explicitly, deliberately well aboveMAX_DISPLAY_FILES: one search feeds all four categories and is narrowed further by the Mine/Shared filters, so a single category must not be starved by the others.getAllOfficeFiles()now returns{ nodes, truncated }, and "Show all in Files" also appears when the search itself was capped — otherwise the page implies it is showing everything when it isn't.The stale
TODOclaiming a server-side cursor/limit API would have to be built first is removed; it already exists.Notes
mainstill passessortingOrder: 'desc'tosortNodes(), andsortNodesinverts the order for mtime (@nextcloud/files:sortingOrder === 'asc' ? 'desc' : 'asc'), so the list is currently displayed oldest-first. fix: sort recent files newest first #79 fixed that and was closed unmerged — both changes are needed for the list to be right.css/main-*.chunk.cssis replaced, since the Vue scoped-style hash changes with the component source. Three older orphaned chunk files predate this branch and are left alone.filterByMimes,categoryMimesandvalidateFilenameare all pure and untested.🤖 Generated with Claude Code