Skip to content

fix: sort recent files newest first - #79

Closed
karlitschek wants to merge 1 commit into
mainfrom
fix/recent-files-sort-order
Closed

fix: sort recent files newest first#79
karlitschek wants to merge 1 commit into
mainfrom
fix/recent-files-sort-order

Conversation

@karlitschek

@karlitschek karlitschek commented Jul 23, 2026

Copy link
Copy Markdown
Member

sortNodes() from @nextcloud/files inverts the sorting order when sortingMode is 'mtime' ('asc' means most recently modified first). Passing 'desc' therefore showed the oldest documents at the top of the Recent list. Pass 'asc' and document the inversion.

Includes rebuilt assets.

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

sortNodes() from @nextcloud/files inverts the sorting order when
sortingMode is 'mtime' ('asc' means most recently modified first).
Passing 'desc' therefore showed the oldest documents at the top of
the Recent list. Pass 'asc' and document the inversion.

Includes rebuilt assets.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@karlitschek
karlitschek requested a review from moodyjmz July 23, 2026 12:49
@karlitschek

Copy link
Copy Markdown
Member Author

@emberfiend

@moodyjmz

Copy link
Copy Markdown
Contributor

TL;DR: Right diagnosis, wrong tree. This branches off 7101a69; main has since had refactor: extract file-filtering logic to utils (3370477), which moved the sort call out of OfficeOverview.vue entirely. The patch here edits code that no longer exists on main, so it's currently a no-op against the actual bug, plus it's unmergeable (CONFLICTING) and DCO is failing (no Signed-off-by:).

Details

The bug diagnosis is correct. Verified directly against node_modules/@nextcloud/files/dist/index.mjs (sortNodes/orderBy): for sortingMode: 'mtime', the library flips the caller's sortingOrder before applying it, so 'asc' → newest-first, 'desc' → oldest-first. main currently passes 'desc' for the Recent list, which is genuinely wrong.

But the live sort call isn't in this diff's target anymore. src/views/OfficeOverview.vue no longer contains an inline sortNodes(...) — it was extracted to src/utils/fileFilters.ts by 3370477, which landed after this PR's merge-base. The actual bug is at src/utils/fileFilters.ts:41 (sortingOrder: 'desc'), untouched here. Merging this as-is changes nothing for users on current main.

There's also already a test that pins the current (buggy) behaviour — src/utils/fileFilters.spec.ts, describe('filterFiles > sort'), with the comment "Surprising, but this is what main already does — characterizing it, not endorsing it." Whoever fixes this needs to flip that assertion too, or the fix will fail CI.

Process note, not a blocker: the rebuilt js//css assets are hand-committed rather than added via the bot's /compile (per .githooks/pre-commit's "never by hand" convention) — CI's build-diff check happens to pass so they're byte-correct for this tree, just outside the normal flow.

Suggested path: close this, redo as a one-liner — src/utils/fileFilters.ts:41 'desc''asc', update the fileFilters.spec.ts sort expectation and its comment, sign off the commit (-s).

@karlitschek

Copy link
Copy Markdown
Member Author

You are completely right. Sorry for the mess with the branches.

@moodyjmz moodyjmz mentioned this pull request Jul 23, 2026
2 tasks
@moodyjmz

Copy link
Copy Markdown
Contributor

Superseded by #85 — rebased onto current main (the code this PR edits has moved to src/utils/fileFilters.ts), with Frank's original diagnosis and fix credited via Co-authored-by:, plus a regression test. Will close this once #85 merges.

@moodyjmz

Copy link
Copy Markdown
Contributor

Implemented in #85

@moodyjmz moodyjmz closed this Jul 23, 2026
moodyjmz added a commit that referenced this pull request Aug 10, 2026
Adds four rules covering what came up while redoing #79/#83 this
session: reuse existing design tokens over hardcoded values, extract
components only for genuine multi-site duplication (as its own
commit unless the component is new to the feature), prefer
@nextcloud/vue over hand-rolled markup, and structure code for
testability without extracting solely to reach a test.

Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
moodyjmz added a commit that referenced this pull request Aug 10, 2026
Every other rule in this file explains its motivating incident in the
commit message or PR body, never inside the instructional text itself
— #79, #83, #84, and #100's own diagnosis are all discussed there, not
quoted in AGENTS.md. "see #100" broke that pattern and adds nothing:
the DAV SEARCH orderby/limit example is already self-contained, and a
bare PR number means nothing to a future reader without the context
this conversation had.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: James Manuel <moodyjmz@users.noreply.github.com>
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