[Bug] Data-view drop down

[TackAdam]

Description

Fixes multiple bugs in the dataset selector where dataset display names and data source titles were not showing correctly.

Bugs Fixed

1. Data Source Showing "datasource" Instead of Actual Title

Issue: When using external data sources, the selector showed "datasource" instead of the actual data source name (e.g., "DockerTest", "PromTest").

Root Cause: convertToDataset() in data_views.ts was using dataSourceRef.name from SavedObjectReference, which is always "dataSource", instead of fetching the actual data source title.

Fix: Modified convertToDataset() to call getDataSource() and populate the actual data source title and version.

Files Changed:

• src/plugins/data/common/data_views/data_views/data_views.ts (lines 746-786)
• src/plugins/data/common/data_views/data_views/data_views.test.ts (added tests at lines 357-460)

2. Dataset Showing Index Pattern Title Instead of Display Name

Issue: Dataset selector showed index pattern title "logs_otel_v1_explore" instead of the saved displayName "LogsExplore".

Root Cause: displayName and description fields were being dropped when loading dataset state from URL parameters.

Fix: Modified redux_persistence.ts to preserve displayName and description when loading from URL state and when creating minimal datasets.

Files Changed:

• src/plugins/explore/public/application/utils/state_management/utils/redux_persistence.ts (lines 76-89, 328-339)
• src/plugins/explore/public/application/utils/state_management/utils/redux_persistence.test.ts (added tests at lines 895-1022)

3. Display Name Lost When Switching Between Datasets

Issue:

• Go to LogsExplore → Refresh (works ✓)
• Switch to Log Dataset → LogsExplore option in dropdown shows "logs_otel_v1_explore" instead of "LogsExplore" (broken ✗)
• Refresh page → Becomes correct again ✓

Root Cause: External data source datasets with composite IDs don't reliably retrieve displayName from saved objects after being cached. When switching to another dataset, the previously selected dataset loses its displayName because it's no longer active and the fetched dataset doesn't have the displayName.

Fix: Implemented a ref-based displayNameCache in dataset_select.tsx that preserves displayNames seen from URL state across dataset switches. The cache is populated when a dataset with displayName is encountered (from URL state or fetched dataView), and these cached values are used when rendering dropdown options.

Files Changed:

• src/plugins/data/public/ui/dataset_select/dataset_select.tsx (added cache at lines 283-286, caching logic at lines 340-346 and 378-384, cache usage at lines 561-569)
• src/plugins/data/public/ui/dataset_select/dataset_select.test.tsx (updated mocks and added test at lines 1014-1065)

Why Both redux_persistence.ts and Caching Are Needed

• redux_persistence.ts: Ensures displayName from URL state reaches the component (the source)
• displayNameCache: Persists displayName across dataset switches (the persistence layer)

Together, they ensure displayNames are preserved throughout the user's session regardless of which dataset is currently selected.

Testing

Added comprehensive test coverage:

• Data source title fetching tests
• DisplayName preservation in URL state tests
• DisplayName caching across dataset switches tests
• All existing tests updated to include displayName fields

Before:

Before.mov

After:

After.mov

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

• All tests pass
  • yarn test:jest
  • yarn test:jest_integration
• New functionality includes testing.
• New functionality has been documented.
• Update CHANGELOG.md
• Commits are signed per the DCO using --signoff

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit a8b20fb)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 Multiple PR themes Sub-PR theme: Fix data source title fetching in convertToDataset Relevant files: src/plugins/data/common/data_views/data_views/data_views.ts src/plugins/data/common/data_views/data_views/data_views.test.ts Sub-PR theme: Preserve displayName and description in redux persistence Relevant files: src/plugins/explore/public/application/utils/state_management/utils/redux_persistence.ts src/plugins/explore/public/application/utils/state_management/utils/redux_persistence.test.ts Sub-PR theme: Fix displayName persistence in dataset selector UI Relevant files: src/plugins/data/public/ui/dataset_select/dataset_select.tsx src/plugins/data/public/ui/dataset_select/dataset_select.test.tsx

⚡ Recommended focus areas for review Cache Staleness The displayNameCache is a useRef that persists for the lifetime of the component. If a user renames a dataset or the displayName changes server-side, the cached value will never be invalidated and will continue to show stale data. There is no mechanism to evict or update cache entries. const displayNameCache = useRef<Map<string, { displayName?: string; description?: string }>>( new Map() ); Precedence Logic The effectiveDisplayName logic gives priority to the cache over the currently selected dataset's displayName. If the selected dataset has a fresher displayName (e.g., updated after a server fetch), the cached (potentially stale) value will still take precedence. Consider whether the selected dataset's displayName should take priority over the cache. const cached = displayNameCache.current.get(id); const effectiveDisplayName = cached?.displayName || (isSelected && selectedDataset?.displayName ? selectedDataset.displayName : displayName); Performance Concern convertToDataset now makes an async network call (getDataSource) every time it is invoked. If this method is called frequently (e.g., for each dataset in a list), it could result in many redundant network requests for the same data source. Consider caching the data source result or batching calls. const dataSource = await this.getDataSource(dataView.dataSourceRef.id); if (dataSource) { // Override with actual data source title and version if available dataSourceInfo = { ...defaultDataSourceInfo, title: dataSource.attributes?.title || defaultDataSourceInfo.title, version: dataSource.attributes?.dataSourceVersion || '', }; } else { // If dataSource is null/undefined, use default dataSourceInfo = defaultDataSourceInfo; } } catch (error) { // If fetching fails, use default dataSourceInfo = defaultDataSourceInfo; } } Duplicate Spread Both urlDataset (lines 78-88) and minimalDataset (lines 323-333) now explicitly list every field including displayName and description. If the Dataset type gains new fields in the future, both blocks will need to be updated. Consider using a spread or a shared helper to construct these objects to reduce duplication and maintenance risk. minimalDataset = { id: selectedDataset.id, title: selectedDataset.title, type: selectedDataset.type, language: selectedDataset.language, timeFieldName: selectedDataset.timeFieldName, dataSource: selectedDataset.dataSource, signalType: selectedDataset.signalType, displayName: selectedDataset.displayName, description: selectedDataset.description, };

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to a8b20fb

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Prevent stale cache from overriding selected dataset display info The cache is populated only when a dataset is selected from URL state, but it is never cleared when a user explicitly selects a different dataset with a different displayName. This means stale cached displayNames could persist and override freshly fetched data for datasets that were previously visited via URL state. Consider clearing or updating the cache entry when a dataset is explicitly selected by the user. src/plugins/data/public/ui/dataset_select/dataset_select.tsx [575-581] const cached = displayNameCache.current.get(id); +// Only use cache if the dataset is not currently selected (to avoid stale data for selected dataset) const effectiveDisplayName = - cached?.displayName || - (isSelected && selectedDataset?.displayName ? selectedDataset.displayName : displayName); + (isSelected ? selectedDataset?.displayName : cached?.displayName) || displayName; const effectiveDescription = - cached?.description || - (isSelected && selectedDataset?.description ? selectedDataset.description : description); + (isSelected ? selectedDataset?.description : cached?.description) || description; Suggestion importance[1-10]: 5 __ Why: The suggestion raises a valid concern about stale cache entries persisting and overriding fresh data. However, the improved_code changes the logic significantly - for the selected dataset it now uses selectedDataset?.displayName directly (which is already set correctly via mergedDataset), and for non-selected datasets uses the cache. This is a reasonable approach but the impact is moderate since the cache is only populated from URL state and the selected dataset's displayName is already correctly set via setSelectedDataset(mergedDataset).	Low
	Log errors instead of silently swallowing them The caught error is silently swallowed with no logging, making it very difficult to diagnose issues in production when getDataSource fails. Consider adding at least a warning log so that failures are observable without breaking the fallback behavior. src/plugins/data/common/data_views/data_views/data_views.ts [951-967] try { const dataSource = await this.getDataSource(dataView.dataSourceRef.id); if (dataSource) { // Override with actual data source title and version if available dataSourceInfo = { ...defaultDataSourceInfo, title: dataSource.attributes?.title || defaultDataSourceInfo.title, version: dataSource.attributes?.dataSourceVersion || '', }; } else { // If dataSource is null/undefined, use default dataSourceInfo = defaultDataSourceInfo; } } catch (error) { - // If fetching fails, use default + // If fetching fails, use default and log a warning + // eslint-disable-next-line no-console + console.warn(`Failed to fetch data source ${dataView.dataSourceRef.id}:`, error); dataSourceInfo = defaultDataSourceInfo; } Suggestion importance[1-10]: 4 __ Why: Adding a warning log for caught errors is a reasonable observability improvement, but using console.warn directly is generally discouraged in this codebase (hence the eslint-disable comment needed). The suggestion is valid but has limited impact and the implementation approach may not align with the project's logging conventions.	Low

---

Previous suggestions

Suggestions up to commit 75ba9bf

Category	Suggestion	Impact
General	Prevent stale cache from overriding non-selected dataset names The cache is populated only when a dataset is selected from URL state, but it is never cleared when a user explicitly selects a different dataset with a different displayName. This means stale cached display names could persist and override freshly fetched display names for datasets that were previously visited via URL state. Consider clearing or updating the cache entry when a dataset is explicitly selected by the user. src/plugins/data/public/ui/dataset_select/dataset_select.tsx [575-581] const cached = displayNameCache.current.get(id); +// Only use cache if the dataset is currently selected (to avoid stale data for non-selected datasets) const effectiveDisplayName = - cached?.displayName || + (isSelected && cached?.displayName) || (isSelected && selectedDataset?.displayName ? selectedDataset.displayName : displayName); const effectiveDescription = - cached?.description || + (isSelected && cached?.description) || (isSelected && selectedDataset?.description ? selectedDataset.description : description); Suggestion importance[1-10]: 5 __ Why: The suggestion identifies a real potential issue where the displayNameCache could serve stale data for non-selected datasets. However, the improved code only partially addresses this by gating cache usage on isSelected, while the original intent of the cache was to show correct names in the dropdown list for all datasets (not just the selected one). The fix may reduce the cache's effectiveness.	Low
	Log errors instead of silently swallowing them The catch block silently swallows all errors without any logging, making it difficult to diagnose issues in production when getDataSource fails. Consider adding at least a warning log so that failures are observable. src/plugins/data/common/data_views/data_views/data_views.ts [951-967] try { const dataSource = await this.getDataSource(dataView.dataSourceRef.id); if (dataSource) { // Override with actual data source title and version if available dataSourceInfo = { ...defaultDataSourceInfo, title: dataSource.attributes?.title || defaultDataSourceInfo.title, version: dataSource.attributes?.dataSourceVersion || '', }; } else { // If dataSource is null/undefined, use default dataSourceInfo = defaultDataSourceInfo; } } catch (error) { // If fetching fails, use default + // eslint-disable-next-line no-console + console.warn(`Failed to fetch data source ${dataView.dataSourceRef.id}:`, error); dataSourceInfo = defaultDataSourceInfo; } Suggestion importance[1-10]: 3 __ Why: Adding a warning log for failed getDataSource calls is a reasonable observability improvement, but it's a minor enhancement that doesn't affect correctness or functionality. The suggestion is valid but low impact.	Low

Suggestions up to commit 1d74ce6

Category	Suggestion	Impact
General	Prevent stale cache values for non-selected datasets The cache is populated only when a dataset is selected from URL state, but it is never cleared when a user explicitly selects a different dataset with a different displayName. This means stale cached display names from a previous URL state could persist and override the correct display names for other datasets after the user navigates away. Consider clearing or updating the cache entry when a dataset is explicitly selected by the user (e.g., in the onSelect handler), or scoping the cache to only apply when the cached entry matches the current URL state dataset. src/plugins/data/public/ui/dataset_select/dataset_select.tsx [575-581] const cached = displayNameCache.current.get(id); +// Only use cached value if it matches the currently selected dataset from URL state const effectiveDisplayName = - cached?.displayName || + (isSelected && cached?.displayName) || (isSelected && selectedDataset?.displayName ? selectedDataset.displayName : displayName); const effectiveDescription = - cached?.description || + (isSelected && cached?.description) || (isSelected && selectedDataset?.description ? selectedDataset.description : description); Suggestion importance[1-10]: 5 __ Why: The suggestion identifies a real potential issue where cached displayName values could persist for non-selected datasets, but the improved_code only partially addresses it by scoping cache usage to isSelected datasets. However, the cache is already primarily useful for the selected dataset scenario, and the suggestion doesn't fully resolve the stale cache problem (cache entries still persist indefinitely). The fix is a reasonable improvement but incomplete.	Low

Suggestions up to commit 1d74ce6

Category	Suggestion	Impact
General	Prevent stale cache from overriding fresh display names The cache is populated only when a dataset is selected from URL state, but it is never cleared when a user explicitly selects a different dataset with a different displayName. This means stale cached display names could persist and override freshly fetched display names for datasets that were previously visited via URL state. Consider clearing a dataset's cache entry when the user explicitly selects it through the UI. src/plugins/data/public/ui/dataset_select/dataset_select.tsx [575-581] const cached = displayNameCache.current.get(id); +// Only use cache if this dataset is NOT the currently selected one, +// or if the selected dataset itself came from URL state (no fresh fetch overrides it) const effectiveDisplayName = - cached?.displayName || - (isSelected && selectedDataset?.displayName ? selectedDataset.displayName : displayName); + (cached?.displayName && (!isSelected || !matchingDataset?.displayName)) + ? cached.displayName + : (isSelected && selectedDataset?.displayName ? selectedDataset.displayName : displayName); const effectiveDescription = - cached?.description || - (isSelected && selectedDataset?.description ? selectedDataset.description : description); + (cached?.description && (!isSelected || !matchingDataset?.description)) + ? cached.description + : (isSelected && selectedDataset?.description ? selectedDataset.description : description); Suggestion importance[1-10]: 5 __ Why: The concern about stale cache entries is valid - once a dataset's displayName is cached from URL state, it will always override the fetched displayName even if the user explicitly selects a different dataset. However, the improved_code references matchingDataset which is not in scope within the options.map callback, making the suggested fix incorrect as written.	Low
	Log errors instead of silently swallowing them The catch block silently swallows all errors without any logging, making it very difficult to diagnose issues in production when getDataSource fails unexpectedly. Consider adding at least a warning log so that failures are observable. src/plugins/data/common/data_views/data_views/data_views.ts [951-967] try { const dataSource = await this.getDataSource(dataView.dataSourceRef.id); if (dataSource) { - // Override with actual data source title and version if available dataSourceInfo = { ...defaultDataSourceInfo, title: dataSource.attributes?.title || defaultDataSourceInfo.title, version: dataSource.attributes?.dataSourceVersion || '', }; } else { - // If dataSource is null/undefined, use default dataSourceInfo = defaultDataSourceInfo; } } catch (error) { // If fetching fails, use default + // eslint-disable-next-line no-console + console.warn(`Failed to fetch data source ${dataView.dataSourceRef.id}:`, error); dataSourceInfo = defaultDataSourceInfo; } Suggestion importance[1-10]: 4 __ Why: Silently swallowing errors in the catch block makes debugging difficult in production. Adding a console.warn is a reasonable improvement for observability, though the impact is minor and the suggestion is straightforward.	Low

Suggestions up to commit 5987728

Category	Suggestion	Impact
Possible issue	Fix misplaced code block causing undefined variable errors There is a misplaced code block that appears to be orphaned — the fetchedDatasets.push(...) block and its closing brace } appear before the datasetPromises map, but reference variables (dataset, dataView) that are only defined inside the map callback below. This will cause a runtime error due to undefined variables. The push logic should be inside the map callback, after convertToDataset is called. src/plugins/data/public/ui/dataset_select/dataset_select.tsx [430-441] - // convertToDataset already includes description, displayName from the saved object - // Just ensure signalType is set from dataView if not already present - fetchedDatasets.push({ - ...dataset, - signalType: dataset.signalType || dataView.signalType, - }); - } // Convert all DataViews to datasets in parallel const datasetPromises = dataViewsArray.map(async (dataView) => { const dataset = await dataViews.convertToDataset(dataView); + // convertToDataset already includes description, displayName from the saved object + // Just ensure signalType is set from dataView if not already present return { + ...dataset, + signalType: dataset.signalType || dataView.signalType, + }; + }); Suggestion importance[1-10]: 8 __ Why: The fetchedDatasets.push(...) block references dataset and dataView variables that are only defined inside the map callback below it, which would cause a runtime error. This is a real bug introduced in the PR that needs to be fixed.	Medium
General	Log errors instead of silently swallowing them The catch block silently swallows all errors without any logging. If getDataSource fails for unexpected reasons (e.g., network errors, permission issues), it will be very difficult to debug since there is no indication of what went wrong. Consider adding at least a warning log in the catch block. src/plugins/data/common/data_views/data_views/data_views.ts [951-967] try { const dataSource = await this.getDataSource(dataView.dataSourceRef.id); if (dataSource) { // Override with actual data source title and version if available dataSourceInfo = { ...defaultDataSourceInfo, title: dataSource.attributes?.title || defaultDataSourceInfo.title, version: dataSource.attributes?.dataSourceVersion || '', }; } else { // If dataSource is null/undefined, use default dataSourceInfo = defaultDataSourceInfo; } } catch (error) { // If fetching fails, use default + // eslint-disable-next-line no-console + console.warn(`Failed to fetch data source ${dataView.dataSourceRef.id}:`, error); dataSourceInfo = defaultDataSourceInfo; } Suggestion importance[1-10]: 3 __ Why: Adding a warning log in the catch block would improve debuggability, but this is a minor improvement since the fallback behavior is correct and the suggestion only adds a console.warn statement.	Low

Suggestions up to commit 97de49c

Category	Suggestion	Impact
General	Cache not applied when building fetched datasets list After this change, displayName and description from the fetched dataset (via convertToDataset) are used directly without merging with any cached URL-state values. This means that if a user had a displayName from URL state cached, it won't be applied to the freshly fetched datasets list, causing the dropdown to show the saved-object name instead of the URL-state name until the selection logic runs. The cache lookup should also be applied here when building fetchedDatasets. src/plugins/data/public/ui/dataset_select/dataset_select.tsx [424-427] fetchedDatasets.push({ ...dataset, signalType: dataset.signalType || dataView.signalType, + // Apply cached displayName/description from URL state if available + ...(displayNameCache.current.has(dataset.id ?? '') && { + displayName: displayNameCache.current.get(dataset.id ?? '')?.displayName || dataset.displayName, + description: displayNameCache.current.get(dataset.id ?? '')?.description || dataset.description, + }), }); Suggestion importance[1-10]: 5 __ Why: This is a valid concern - the displayNameCache is populated but not consulted when building fetchedDatasets, which could cause the dropdown to briefly show stale names. The suggested fix is logically sound and addresses a real gap in the implementation.	Low
	Stale cache entries never invalidated The displayNameCache ref is initialized with a new Map() inside the component, but since useRef only initializes once, this is fine for a single mount. However, the cache is never cleared when datasets change or when the component re-renders with new data. This could cause stale display names to persist indefinitely if a dataset's display name is updated externally. Consider adding a mechanism to invalidate or update cache entries when fresh data is fetched. src/plugins/data/public/ui/dataset_select/dataset_select.tsx [283-286] // Cache displayNames we've seen from URL state so they persist when switching datasets +// Cache is keyed by dataset id; entries are updated when fresh URL state is observed const displayNameCache = useRef<Map<string, { displayName?: string; description?: string }>>( new Map() ); Suggestion importance[1-10]: 3 __ Why: The suggestion points out a valid concern about stale cache entries, but the improved_code is nearly identical to the existing_code (only adds a comment), making it a documentation-only change. The actual fix (cache invalidation logic) is not implemented in the improved_code.	Low
	Silent error swallowing hinders debugging The caught error is silently swallowed with no logging, making it very difficult to diagnose issues in production when getDataSource fails. At minimum, a warning should be logged so that unexpected failures (e.g., network errors, permission issues) are visible during debugging. src/plugins/data/common/data_views/data_views/data_views.ts [762-778] try { const dataSource = await this.getDataSource(dataView.dataSourceRef.id); if (dataSource) { // Override with actual data source title and version if available dataSourceInfo = { ...defaultDataSourceInfo, title: dataSource.attributes?.title || defaultDataSourceInfo.title, version: dataSource.attributes?.dataSourceVersion || '', }; } else { // If dataSource is null/undefined, use default dataSourceInfo = defaultDataSourceInfo; } } catch (error) { // If fetching fails, use default + // eslint-disable-next-line no-console + console.warn(`Failed to fetch data source for id ${dataView.dataSourceRef.id}:`, error); dataSourceInfo = defaultDataSourceInfo; } Suggestion importance[1-10]: 3 __ Why: Adding a console.warn for silent error swallowing is a minor improvement for debuggability, but this is a low-impact style/maintainability suggestion that doesn't affect correctness or functionality.	Low

[github-actions]

Persistent review updated to latest commit 09465b5

[github-actions]

Persistent review updated to latest commit 09465b5

[github-actions]

Persistent review updated to latest commit d53e171

[codecov]

Codecov Report

❌ Patch coverage is 77.41935% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.23%. Comparing base (e4ddbfe) to head (a8b20fb).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
...s/data/public/ui/dataset_select/dataset_select.tsx	78.94%	3 Missing and 1 partial ⚠️
...ns/data/common/data_views/data_views/data_views.ts	75.00%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11392      +/-   ##
==========================================
+ Coverage   61.21%   61.23%   +0.01%     
==========================================
  Files        4985     4985              
  Lines      136862   136886      +24     
  Branches    23889    23902      +13     
==========================================
+ Hits        83786    83823      +37     
+ Misses      47072    47059      -13     
  Partials     6004     6004              

Flag	Coverage Δ
Linux_1	24.79% <0.00%> (-0.01%)	⬇️
Linux_2	39.34% <0.00%> (-0.01%)	⬇️
Linux_3	42.63% <77.41%> (+0.01%)	⬆️
Linux_4	33.57% <0.00%> (-0.01%)	⬇️
Windows_1	24.81% <0.00%> (-0.01%)	⬇️
Windows_2	39.32% <0.00%> (-0.01%)	⬇️
Windows_3	42.64% <77.41%> (?)
Windows_4	33.58% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mengweieric]

We can move the common fields into a default dataSourceInfo object to reduce duplication.

[TackAdam]

Thanks for suggestion,
Updated to use

      const defaultDataSourceInfo = {
        id: dataView.dataSourceRef.id,
        title: dataView.dataSourceRef.id,
        type: dataView.dataSourceRef.type || DEFAULT_DATA.SOURCE_TYPES.OPENSEARCH,
        version: '',
      };
      ```

[github-actions]

Persistent review updated to latest commit d086df0

[github-actions]

Persistent review updated to latest commit d086df0

[github-actions]

Persistent review updated to latest commit c860a7d

[github-actions]

Persistent review updated to latest commit 97de49c

[github-actions]

Persistent review updated to latest commit 5987728

[github-actions]

Persistent review updated to latest commit 1d74ce6

[github-actions]

Persistent review updated to latest commit 1d74ce6

[github-actions]

Persistent review updated to latest commit 75ba9bf

[github-actions]

Persistent review updated to latest commit a8b20fb