Skip to content

Languages: Page through to load all configured languages - #22765

Merged
iOvergaard merged 3 commits into
v17/devfrom
v17/bugfix/ensure-all-languages-loaded
Jun 15, 2026
Merged

Languages: Page through to load all configured languages#22765
iOvergaard merged 3 commits into
v17/devfrom
v17/bugfix/ensure-all-languages-loaded

Conversation

@AndyButland

@AndyButland AndyButland commented May 8, 2026

Copy link
Copy Markdown
Contributor

Description

This PR is a follow-up to review feedback on #22711. The global content language selector — and twelve other places — called UmbLanguageCollectionRepository.requestCollection({}), which falls through to the server's default take = 100. Installations with more than 100 configured languages would silently lose the rest.

In practice this seems very unlikely, so this update is likely an academic one, but nonetheless, as per the comment here, we should:

At a minimum, we need to ensure that all languages are loaded on the client by comparing the received amount with the total and continuing to request until we have them all.

As this seemed it could be a more general requirement, I've introduced a generic fetchAllPages<T> utility under src/packages/core/utils/pagination/offset/ that pages through any offset-paginated UmbDataSourceResponse<UmbPagedModel<T>> source until total is reached.

I've then added UmbLanguageCollectionRepository.requestAllItems() on the language collection repository, delegating to fetchAllPages with a page size of 100. And migrated all call sites across language, dictionary, multi-url-picker, preview, content, and document publish/unpublish actions from requestCollection({}) (or workarounds like take: 999 / take: 1000) to requestAllItems().

Testing

Unit test are added for fetchAllPages.

To manually test, temporarily change the page size at src/Umbraco.Web.UI.Client/src/packages/language/collection/repository/language-collection.repository.ts, line 28 from 100 to a smaller value, that's less that the number of languages you have. Verify that all places where a list of languages are shown, that you see them all.

…where the number of languages exceeds the default page size.
Copilot AI review requested due to automatic review settings May 8, 2026 07:31
@claude

claude Bot commented May 8, 2026

Copy link
Copy Markdown

Claude finished @AndyButland's task in 3m 33s —— View job


PR Review

Target: origin/main · Based on commit: 170d3a80

Adds a generic fetchAllPages utility that pages through any offset-paginated source, adds requestAllItems() to UmbLanguageCollectionRepository, and migrates 13 call sites away from unbounded requestCollection({}) / magic-number workarounds.

  • Modified public API: New fetchAllPages<T> function and UmbOffsetPageFetcher<T> type exported via @umbraco-cms/backoffice/utils; new requestAllItems() method on UmbLanguageCollectionRepository
  • Affected implementations (outside this PR): None — all public symbols are new additions

Suggestions

  • fetch-all-pages.function.ts:44: total: allItems.length is returned rather than the server-reported total variable. In the normal path these are always equal, but in the defensive-break scenario (empty-page guard fires) they can differ — e.g. server says total: 10 but only 2 items were returned; the caller gets back total: 2. This is probably the right behavior (returning what was actually collected), but it's not explicitly documented in the JSDoc. A one-liner noting "if the defensive guard fires, the returned total reflects the actual items collected, not the server-reported total" would make the contract explicit.

  • fetch-all-pages.function.test.ts:89: The defensive-break test (breaks out if the server returns an empty page despite reporting a higher total) asserts data?.items.length and callCount, but doesn't assert data?.total. Adding expect(data?.total).to.equal(2) would pin down the contract (total = actual items collected, not server-reported 10) and prevent a future refactor from silently changing it.

  • language-collection.repository.ts:30: Page size 100 is hardcoded as a magic literal. Since it mirrors the server's default and is intentional, extracting it to a named constant (e.g. const PAGE_SIZE = 100) at the top of the file — or even just a comment — would make the choice visible to future readers and ease updates.


Approved with Suggestions for improvement

Good to go, but please carefully consider the importance of the suggestions.

The fix is well-scoped: the utility is generic, correctly handles all edge cases (single page, multi-page, exact-fill, empty, mid-page error, empty-page guard), is fully tested, and all 13 call sites have been consistently migrated including the previous take: 999 / take: 1000 workarounds. No breaking changes.

@claude claude Bot added the area/frontend label May 8, 2026

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

This PR ensures the backoffice always loads the full configured language set (even when there are more than the server default page size of 100) by introducing a reusable offset-pagination “fetch all pages” utility and migrating existing call sites to use a new requestAllItems() repository API.

Changes:

  • Added a generic fetchAllPages<T> helper for offset-paginated UmbPagedModel<T> sources (with unit tests).
  • Added UmbLanguageCollectionRepository.requestAllItems() to page through and return the full language set.
  • Updated multiple UI contexts/actions to replace single-page requestCollection({}) (and take: 999/1000 workarounds) with requestAllItems().

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Umbraco.Web.UI.Client/src/packages/preview/preview-apps/preview-culture.element.ts Loads cultures via full language retrieval to avoid truncation past 100.
src/Umbraco.Web.UI.Client/src/packages/multi-url-picker/document-link-picker-modal/document-link-picker.context.ts Loads the complete language list for the link picker modal.
src/Umbraco.Web.UI.Client/src/packages/language/modals/language-picker/language-picker-modal.element.ts Ensures language picker modal receives all configured languages.
src/Umbraco.Web.UI.Client/src/packages/language/global-contexts/app-language.context.ts App language global context now pulls the full language set.
src/Umbraco.Web.UI.Client/src/packages/language/collection/repository/language-collection.repository.ts Adds requestAllItems() that pages through language collection results.
src/Umbraco.Web.UI.Client/src/packages/language/app-language-select/app-language-select.element.ts App language selector now uses full language retrieval.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/publishing/unpublish/entity-bulk-action/unpublish.bulk-action.ts Uses full language list during unpublish bulk action flows.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/publishing/unpublish/entity-action/unpublish.action.ts Uses full language list during unpublish single action flows.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/publishing/publish/entity-bulk-action/publish.bulk-action.ts Uses full language list during publish bulk action flows.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/publishing/publish/entity-action/publish.action.ts Uses full language list during publish single action flows.
src/Umbraco.Web.UI.Client/src/packages/documents/documents/entity-actions/culture-and-hostnames/modal/culture-and-hostnames-modal.element.ts Removes take: 999 workaround by paging all languages.
src/Umbraco.Web.UI.Client/src/packages/dictionary/workspace/views/workspace-view-dictionary-editor.element.ts Ensures dictionary editor view gets all languages.
src/Umbraco.Web.UI.Client/src/packages/dictionary/collection/views/table/dictionary-table-collection-view.element.ts Ensures dictionary table view columns cover all languages.
src/Umbraco.Web.UI.Client/src/packages/core/utils/pagination/offset/index.ts Exposes the new pagination helper from the offset pagination utils barrel.
src/Umbraco.Web.UI.Client/src/packages/core/utils/pagination/offset/fetch-all-pages.function.ts Implements generic offset “fetch all pages” accumulation logic.
src/Umbraco.Web.UI.Client/src/packages/core/utils/pagination/offset/fetch-all-pages.function.test.ts Adds unit tests covering paging scenarios and failure/safeguard behavior.
src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-detail-workspace-base.ts Content workspace language loading now retrieves the full language set.

@iOvergaard
iOvergaard merged commit 828e359 into v17/dev Jun 15, 2026
29 of 30 checks passed
@iOvergaard
iOvergaard deleted the v17/bugfix/ensure-all-languages-loaded branch June 15, 2026 14:17
@iOvergaard iOvergaard added category/performance Fixes for performance (generally cpu or memory) fixes type/feature release/17.5.0 release/18.1.0 labels Jun 15, 2026
leekelleher pushed a commit that referenced this pull request Jun 16, 2026
…t check (#23133)

Backoffice: Move fetchAllPages into the repository module to break a core circular import

#22765 added the offset pagination helper `fetchAllPages` under
`@umbraco-cms/backoffice/utils`, but its contract is expressed entirely in
repository-owned types (`UmbDataSourceResponse<UmbPagedModel<T>>`). That made
`utils` import `repository` while `repository` already imports `utils`,
introducing a 17th core bidirectional module import and tripping
`check:module-dependencies` (threshold 16) — failing the `test` job on every
open PR.

Relocate the helper (and its test) into the `repository` module, which
legitimately owns those types, and export it from
`@umbraco-cms/backoffice/repository`. The sole consumer
(UmbLanguageCollectionRepository) already imports from that module. Core
bidirectional imports are back to 16.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/frontend category/performance Fixes for performance (generally cpu or memory) fixes release/17.6.0 release/18.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants