Skip to content

Entity Data Picker: Fix "Not Found" in remove dialog for entities without a top-level name#22915

Merged
leekelleher merged 7 commits into
release/17.5.0from
v17/hotfix/entity-data-picker-resolve-item-name
May 21, 2026
Merged

Entity Data Picker: Fix "Not Found" in remove dialog for entities without a top-level name#22915
leekelleher merged 7 commits into
release/17.5.0from
v17/hotfix/entity-data-picker-resolve-item-name

Conversation

@madsrasmussen

Copy link
Copy Markdown
Member

Root cause:

The base _requestItemName reads item.name. Document entities store their names in variants[x].name (language-context-dependent) and have no top-level name property, so the remove confirmation dialog always showed "Not Found".

Summary

  • Adds an optional createItemDataResolver? factory method to UmbPickerDataSource — data sources can implement it to control how a picked item's display name (and other metadata) is resolved
  • UmbEntityDataPickerInputContext overrides _requestItemName to call the factory when available, passing itself as the controller host so the resolver can reach local DOM contexts such as UMB_VARIANT_CONTEXT
  • Updates the ExampleDocumentPickerPropertyEditorDataSource example to implement createItemDataResolver using the existing UmbDocumentItemDataResolver

Test plan

  • Run example "picker-data-source" with npm run example
  • Set up a Data Type with a Entity Data Picker Property Editor.
  • Choose "Documents" as the Data Source.
    • Pick a document via the Entity Data Picker, click Remove — the dialog should show the document's display name, not "Not Found"
  • Verify the same with a multilingual site: the name shown should match the active language context
  • Data source without createItemDataResolver — falls back to item.name (existing behaviour, unaffected)
  • Unit tests: npm test -- --files "src/packages/property-editors/entity-data-picker/input/input-entity-data.context.test.ts" — 4 tests covering the resolver path, no-resolver fallback, resolver-returns-undefined fallback, and not-found case

Copilot AI review requested due to automatic review settings May 20, 2026 19:34
@claude

claude Bot commented May 20, 2026

Copy link
Copy Markdown

Claude finished @madsrasmussen's task in 5m 4s —— View job


PR Review

Target: origin/release/17.5.0 · Based on commit: 8edf80a5 · Skipped: 0 noise files out of 4 total

Generalises the variant-aware name resolution that already existed in input-document.context.ts into a pluggable createItemDataResolver? strategy on UmbPickerDataSource, then wires it into UmbEntityDataPickerInputContext._requestItemName so any entity type can supply its own resolver. Fixes the "Not Found" shown in the remove-dialog for documents, whose display names live in variants[n].name, not on a top-level name property.

  • Modified public API: UmbPickerDataSource (new optional method createItemDataResolver?)
  • Other changes: Remove dialog in UmbEntityDataPickerInputContext now shows the correct language-context-aware display name when the data source implements createItemDataResolver; falls back transparently to item.name otherwise

Suggestions

  • src/Umbraco.Web.UI.Client/src/packages/property-editors/entity-data-picker/input/input-entity-data.context.ts:57: _requestItemName creates a brand-new resolver instance on every invocation. For the single-item remove dialog this is fine — the same pattern is already used in input-document.context.ts. If this method is ever called in bulk (e.g. a future "remove all selected" flow), the per-call allocation would create many short-lived controllers. Not a problem today; just worth keeping in mind if bulk-remove is ever added.

  • src/Umbraco.Web.UI.Client/src/packages/property-editors/entity-data-picker/input/input-entity-data.context.test.ts:71: The comment on requestCollection()"requestCollection is required so isPickerCollectionDataSource returns true, allowing #setModalToken to resolve without throwing" — is correct and useful. One small note: setDataSourceApi calls #setModalToken() without await, so a throw from it becomes an unhandled rejection rather than a test failure. The comment could optionally note this nuance: "prevents unhandled rejection from #setModalToken when setDataSourceApi is called in tests." Entirely optional — the existing comment is already informative.


Approved

Clean, well-tested fix. The optional strategy on UmbPickerDataSource is a non-breaking extension of the public interface, the resolver lifecycle (create → setData → getName → removeUmbController) mirrors the established pattern in input-document.context.ts, and the four unit tests cover resolver path, no-resolver fallback, resolver-returns-undefined fallback, and the not-found case. Good to go.

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 fixes the “Not Found” label in the Entity Data Picker remove confirmation dialog for entity types (notably Documents) whose display name is not stored on a top-level item.name, by allowing the active picker data source to provide an item data resolver for display-name resolution.

Changes:

  • Adds an optional createItemDataResolver(host) factory method to UmbPickerDataSource so data sources can control how picked item metadata (e.g., display name) is resolved.
  • Updates UmbEntityDataPickerInputContext to use the data source’s resolver (when available) to resolve the remove-dialog display name.
  • Adds unit tests covering resolver usage and fallback behavior; updates the example document picker data source to supply UmbDocumentItemDataResolver.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/Umbraco.Web.UI.Client/src/packages/property-editors/entity-data-picker/input/input-entity-data.context.ts Uses a data-source-provided resolver to compute the remove-dialog item name, enabling variant/language-aware names.
src/Umbraco.Web.UI.Client/src/packages/property-editors/entity-data-picker/input/input-entity-data.context.test.ts Adds tests validating resolver path + fallback behaviors for _requestItemName.
src/Umbraco.Web.UI.Client/src/packages/core/picker-data-source/data-source/types.ts Extends the picker data source contract with an optional resolver factory.
src/Umbraco.Web.UI.Client/examples/picker-data-source/example-document-picker-data-source.ts Demonstrates createItemDataResolver using UmbDocumentItemDataResolver for document names/icons.

@madsrasmussen madsrasmussen requested a review from leekelleher May 21, 2026 07:24
@leekelleher leekelleher merged commit c74a582 into release/17.5.0 May 21, 2026
28 checks passed
@leekelleher leekelleher deleted the v17/hotfix/entity-data-picker-resolve-item-name branch May 21, 2026 08:13
leekelleher pushed a commit that referenced this pull request May 21, 2026
…hout a top-level name (#22915)

* Add item data resolver support to picker data sources

* add js docs

* remove duplicated fallback logic

* wip unit tests of requestItemName method

* Use DocumentVariantStateModel in mock documents to fix compiler

* Update input-entity-data.context.ts

* Update input-entity-data.context.test.ts

(cherry picked from commit c74a582)
iOvergaard pushed a commit that referenced this pull request May 21, 2026
* Element Tree Picker Data Source: adds item data resolver

This follows PR #22915, which fixes the Entity Data Picker's
removal confirmation message with the entity's name.

* Adds support for `UmbElementFolderItemDataResolver`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants