Skip to content

fix(UVE Palette): Add sorting parameters to content retrieval#35774

Queued
rjvelazco wants to merge 1 commit into
mainfrom
issue-35770-page-editor-newest-content-appearing-last-in-content-lists-on-left-component-panel
Queued

fix(UVE Palette): Add sorting parameters to content retrieval#35774
rjvelazco wants to merge 1 commit into
mainfrom
issue-35770-page-editor-newest-content-appearing-last-in-content-lists-on-left-component-panel

Conversation

@rjvelazco
Copy link
Copy Markdown
Member

@rjvelazco rjvelazco commented May 20, 2026

Video

video.mov

This PR fixes: #35770

@rjvelazco rjvelazco enabled auto-merge May 20, 2026 16:54
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 20, 2026

Claude finished @rjvelazco's task in 1m 53s —— View job


Review

The fix is correct and minimal. The interesting context is in DotESContentService — it already defaults to 'modDate' / 'DESC' (dot-es-content.service.ts:36-37), but because the service holds those as instance state and setBaseParams only overwrites them when the caller passes a truthy value (this._sortField = params.sortField || this._sortField), any prior caller in the app that used a different sort would leak that state into the palette's next call. Explicitly setting the params here makes the behavior deterministic regardless of other callers — good defensive fix. ✅

A few small things worth a second look:

1. Stale JSDoc example — utils/index.ts:356-362

The @example block in buildESContentParams still shows the old return shape and is now out of sync with the actual return type:

// Returns: {
//   query: '+contentType:Blog +deleted:false +variant:DEFAULT',
//   offset: '30',
//   itemsPerPage: 30,
//   lang: '1',
//   filter: 'news'
//   // ← missing sortField/sortOrder
// }

2. Prefer the existing enum over the string literal — utils/index.ts:377-378, 389-390

ESOrderDirection is already exported from @dotcms/data-access and is the type used by queryEsParams.sortOrder. Typing the return as sortOrder: ESOrderDirection (and using ESOrderDirection.DESC at the call site) would be more consistent with the service contract and prevent accidental typos like 'desc'.

import { ESOrderDirection } from '@dotcms/data-access';
// ...
sortOrder: ESOrderDirection;
// ...
sortOrder: ESOrderDirection.DESC

Fix these →

3. Nit — hardcoded 'modDate'

The intent of the fix is "always sort by latest modified". Since the field name is duplicated between the impl and the test, a named constant (const PALETTE_SORT_FIELD = 'modDate') would make the intent explicit and keep the test honest. Optional.

4. Test only re-asserts the literal

utils.spec.ts adds "should always sort by modDate DESC" but really just re-checks the hardcoded value. The more valuable assertion already exists implicitly in store.spec.ts:690-696 (the params passed into dotESContentService.get). The new test isn't harmful, but it doesn't add coverage beyond what's already there.


Nothing here is blocking — overall the change is small, correct, and well-scoped to the reported issue.

  • Read the changed files
  • Analyze the diff against origin/main
  • Check tests for adequacy
  • Post review findings
    · Branch

@github-actions github-actions Bot added the Area : Frontend PR changes Angular/TypeScript frontend code label May 20, 2026
@rjvelazco rjvelazco added this pull request to the merge queue May 20, 2026
Any commits made after this event will not be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Page editor Newest content appearing last in content lists on left component panel

3 participants