Skip to content

[Discover] Managed data view improvement#223451

Merged
christineweng merged 17 commits intoelastic:mainfrom
christineweng:managed-dataview-disable-save
Sep 22, 2025
Merged

[Discover] Managed data view improvement#223451
christineweng merged 17 commits intoelastic:mainfrom
christineweng:managed-dataview-disable-save

Conversation

@christineweng
Copy link
Copy Markdown
Contributor

@christineweng christineweng commented Jun 11, 2025

Summary

This PR adds a managed property to DataView type. It leveraged the existing managed property in saved objects, when a data view is created, managed is also passed to SO creation.

Major changes:

  • Kibana managed data views (from integrations, discover and security) now have a Managed tag. The tags are present consistently across applications
  • When viewing an existing data view, user is able to duplicate the data view via the new Duplicate button
  • When clicking Managed this data view for a managed data view, the form is disabled from editing. User has the option to click Duplicate, which will open a new flyout with the indices populated
  • New fields cannot be saved in a managed data view
  • In Stack management -> Data views, editing a managed data view also shows a disabled form. Managed tag is added.
  • Out of the scope of this PR: Duplicate is not available in stack management -> data views -> edit. If the data view is managed, the form is disabled and save button is not present
Screen.Recording.2025-09-04.at.2.18.03.PM.mov

Data view editor flyout logic

As the variation of the editor form increase from 2 scenarios to 4, additional logic is added. A small refactoring was made to onDataViewCreated to standardized the props, so that it can be reused by the duplicate logic (when duplicating, it is essentially the same behavior as creating a data view.

image

UI updates

image image image

Checklist

  • Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
  • Documentation was added for features that require explanation or tutorials
  • Unit or functional tests were updated or added to match the most common scenarios
  • If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
  • This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
  • Flaky Test Runner was used on any tests changed
  • The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

christineweng added a commit that referenced this pull request Jun 24, 2025
## Summary

Ref: elastic/security-team#12791

Enable `newDataViewPickerEnabled` and fresh kibana build

### Before

![image](https://github.com/user-attachments/assets/f0047b00-ce26-49f9-827c-44a07538a12a)


### After

Label is cut off in security pages because of the `Managed` label, will
address this in a separate PR (likely in
#223451)


![image](https://github.com/user-attachments/assets/f79615aa-05be-4d9a-8e33-4eca5c82591c)


![image](https://github.com/user-attachments/assets/dc7a4c23-c17c-4bb6-b1ca-d778900e5265)


### Checklist

- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 24, 2025
## Summary

Ref: elastic/security-team#12791

Enable `newDataViewPickerEnabled` and fresh kibana build

### Before

![image](https://github.com/user-attachments/assets/f0047b00-ce26-49f9-827c-44a07538a12a)

### After

Label is cut off in security pages because of the `Managed` label, will
address this in a separate PR (likely in
elastic#223451)

![image](https://github.com/user-attachments/assets/f79615aa-05be-4d9a-8e33-4eca5c82591c)

![image](https://github.com/user-attachments/assets/dc7a4c23-c17c-4bb6-b1ca-d778900e5265)

### Checklist

- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

(cherry picked from commit 26a14f3)
kibanamachine added a commit that referenced this pull request Jun 24, 2025
…) (#225174)

# Backport

This will backport the following commits from `main` to `8.19`:
- [[Security Solution] Fix default data view name mismatch
(#224333)](#224333)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT
[{"author":{"name":"christineweng","email":"18648970+christineweng@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-06-24T21:05:52Z","message":"[Security
Solution] Fix default data view name mismatch (#224333)\n\n##
Summary\n\nRef:
https://github.com/elastic/security-team/issues/12791\n\nEnable
`newDataViewPickerEnabled` and fresh kibana build\n\n###
Before\n\n![image](https://github.com/user-attachments/assets/f0047b00-ce26-49f9-827c-44a07538a12a)\n\n\n###
After\n\nLabel is cut off in security pages because of the `Managed`
label, will\naddress this in a separate PR (likely
in\nhttps://github.com//pull/223451)\n\n\n![image](https://github.com/user-attachments/assets/f79615aa-05be-4d9a-8e33-4eca5c82591c)\n\n\n![image](https://github.com/user-attachments/assets/dc7a4c23-c17c-4bb6-b1ca-d778900e5265)\n\n\n###
Checklist\n\n- [x] The PR description includes the appropriate Release
Notes section,\nand the correct `release_note:*` label is applied per
the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"26a14f36bf8208caa8836b54607514810378372b","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Threat
Hunting:Investigations","backport:version","v9.1.0","v8.19.0"],"title":"[Security
Solution] Fix default data view name
mismatch","number":224333,"url":"https://github.com/elastic/kibana/pull/224333","mergeCommit":{"message":"[Security
Solution] Fix default data view name mismatch (#224333)\n\n##
Summary\n\nRef:
https://github.com/elastic/security-team/issues/12791\n\nEnable
`newDataViewPickerEnabled` and fresh kibana build\n\n###
Before\n\n![image](https://github.com/user-attachments/assets/f0047b00-ce26-49f9-827c-44a07538a12a)\n\n\n###
After\n\nLabel is cut off in security pages because of the `Managed`
label, will\naddress this in a separate PR (likely
in\nhttps://github.com//pull/223451)\n\n\n![image](https://github.com/user-attachments/assets/f79615aa-05be-4d9a-8e33-4eca5c82591c)\n\n\n![image](https://github.com/user-attachments/assets/dc7a4c23-c17c-4bb6-b1ca-d778900e5265)\n\n\n###
Checklist\n\n- [x] The PR description includes the appropriate Release
Notes section,\nand the correct `release_note:*` label is applied per
the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"26a14f36bf8208caa8836b54607514810378372b"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/224333","number":224333,"mergeCommit":{"message":"[Security
Solution] Fix default data view name mismatch (#224333)\n\n##
Summary\n\nRef:
https://github.com/elastic/security-team/issues/12791\n\nEnable
`newDataViewPickerEnabled` and fresh kibana build\n\n###
Before\n\n![image](https://github.com/user-attachments/assets/f0047b00-ce26-49f9-827c-44a07538a12a)\n\n\n###
After\n\nLabel is cut off in security pages because of the `Managed`
label, will\naddress this in a separate PR (likely
in\nhttps://github.com//pull/223451)\n\n\n![image](https://github.com/user-attachments/assets/f79615aa-05be-4d9a-8e33-4eca5c82591c)\n\n\n![image](https://github.com/user-attachments/assets/dc7a4c23-c17c-4bb6-b1ca-d778900e5265)\n\n\n###
Checklist\n\n- [x] The PR description includes the appropriate Release
Notes section,\nand the correct `release_note:*` label is applied per
the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"26a14f36bf8208caa8836b54607514810378372b"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: christineweng <18648970+christineweng@users.noreply.github.com>
akowalska622 pushed a commit to akowalska622/kibana that referenced this pull request Jun 25, 2025
## Summary

Ref: elastic/security-team#12791

Enable `newDataViewPickerEnabled` and fresh kibana build

### Before

![image](https://github.com/user-attachments/assets/f0047b00-ce26-49f9-827c-44a07538a12a)


### After

Label is cut off in security pages because of the `Managed` label, will
address this in a separate PR (likely in
elastic#223451)


![image](https://github.com/user-attachments/assets/f79615aa-05be-4d9a-8e33-4eca5c82591c)


![image](https://github.com/user-attachments/assets/dc7a4c23-c17c-4bb6-b1ca-d778900e5265)


### Checklist

- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
@christineweng christineweng force-pushed the managed-dataview-disable-save branch 2 times, most recently from 9aceec8 to ddeaf14 Compare July 15, 2025 22:00
@christineweng christineweng changed the title disable save in managed data view [Discover] Managed data view improvement Jul 15, 2025
data_view: serviceKey === SERVICE_KEY ? dataViewSpecSchema : schema.never(),
index_pattern:
serviceKey === SERVICE_KEY_LEGACY ? dataViewSpecSchema : schema.never(),
managed: schema.maybe(schema.boolean({ defaultValue: false })),
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.

@davismcphee who would we need to update on the docs side about documenting this change in a release note? Not sure what the process is for platform here..

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.

I don't think we should expose managed in the public API. Is it needed? I'm under the impression all internal requests run through the content management API that uses the saved object client under the hood.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too familiar with internal/external API, is removing this line sufficient to keep it internal?

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.

Yep, should be. Sorry for the delayed response here!

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.

I think it should be, and maybe removing from the schema in src/platform/plugins/shared/data_views/server/rest_api_routes/schema.ts too.

const savedDataViewRefs = savedDataViews
const availableDataViewRefs = savedDataViews
? savedDataViews
: (await data.dataViews.getIdsWithTitle()) ?? [];
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.

Not really on you here, but should this be wrapped in a try catch? fyi @davismcphee

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.

More of a question for the Presentation team as Unified Search owners, but it definitely could be. Even something like data.dataViews.getIdsWithTitle().catch(() => []) would stop things from blowing up, although I guess it must not fail much as is or we'd probably hear about it 😄

Copy link
Copy Markdown
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! The code changes are looking good overall. I left some questions and feedback in addition to what @michaelolo24 already added, but it seems like we're on the right track. I have a few comments around the UX, but I'll save them for the One Discover sync so we can discuss.

A couple of things I noticed when testing locally:

A user is still able to implicitly modify a managed data view through the field editor flyout and persist the changes. I think for this case, we can probably just do a check for managed and completely disable the ability to save, with a tooltip along the lines of Fields cannot be edited on managed data views. Duplicate the data view in order to make changes.:

It looks like managed data views can still be edited from data view management. This might be related to the isEdit thing added in this PR. We may need to figure out a way to handle this without each data view editor consumer having to explicitly pass isEdit since it can be error prone if missed:

const savedDataViewRefs = savedDataViews
const availableDataViewRefs = savedDataViews
? savedDataViews
: (await data.dataViews.getIdsWithTitle()) ?? [];
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.

More of a question for the Presentation team as Unified Search owners, but it definitely could be. Even something like data.dataViews.getIdsWithTitle().catch(() => []) would stop things from blowing up, although I guess it must not fail much as is or we'd probably hear about it 😄

name: 'All logs',
title: 'logs-*,dataset-logs-*-*',
timeFieldName: '@timestamp',
managed: true,
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.

On a totally unrelated note since I just happened to notice it, @elastic/obs-ux-logs-team shouldn't this just be { id: ALL_LOGS_DATA_VIEW_ID } like it is here?

Copy link
Copy Markdown
Contributor

@gbamparop gbamparop Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thomheymann @weltenwort is this a leftover from #209565? Or we need this for redirects?

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.

@davismcphee Yeh, looks like All Logs is used as a fallback which should be using the ALL_LOGS_DATA_VIEW_ID data view.

@gbamparop This code was introduced as part of #215867 to pass app state from Logs Stream component to Discover.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Although I suspect this is a dead code path since the conditional earlier in that function already seems to cover all cases and we'd never reach that fall-through. For cleanliness's sake one could turn the 'all' branch into the default to remove this.

@christineweng christineweng force-pushed the managed-dataview-disable-save branch 3 times, most recently from ee414c0 to 1a3bbd8 Compare July 29, 2025 21:10
@christineweng christineweng self-assigned this Jul 29, 2025
@christineweng christineweng added release_note:fix Team:DataDiscovery Discover, search (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// Team:Threat Hunting:Investigations Security Solution Threat Hunting Investigations Team v9.2.0 backport:skip This PR does not require backporting labels Jul 29, 2025
@christineweng christineweng force-pushed the managed-dataview-disable-save branch from 1a3bbd8 to f4a8630 Compare July 30, 2025 02:15
Copy link
Copy Markdown
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one more round of minor feedback, but we're just about there! I've tested basically everything I could think of at this point and it's working great overall, so this should be good to go on my end once addressed 🙂

onSave: (newDataView) => {
onDataViewCreated(newDataView);
},
allowAdHocDataView: true,
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.

So I ended up looking into this, and afaict there weren't any consumers who both allowed data view creation within Unified Search but didn't support ad hoc data views, so I'm pretty sure we're ok here. But I'll ping them again directly in their channel to be sure.

Copy link
Copy Markdown
Member

@florent-leborgne florent-leborgne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated copy LGTM

@christineweng christineweng force-pushed the managed-dataview-disable-save branch from 1b769db to 4b4a35a Compare September 11, 2025 18:50
Copy link
Copy Markdown
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the latest changes, this all looks good to me now! I opened a tiny PR against the branch to fix the data views API issue, but once that's merged and we confirm CI is green, I'll drop by to approve 🙂

Copy link
Copy Markdown
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @christineweng! Thanks for all your effort here and working with us through the iterations 🙏 It was a long road, but I think we're finally there!

@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Sep 22, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [0390bf5]

History

cc @christineweng

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Data Views Data Views code and UI - index patterns before 8.0 needs_docs release_note:enhancement Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. Team:DataDiscovery Discover, search (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// Team:Threat Hunting:Investigations Security Solution Threat Hunting Investigations Team v9.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.