Saved-object import warnings

[kobelb]

As part of the effort to allow saved-objects with encrypted attributes to be exported/imported, we've decided that we should allow developers to provide custom warnings that are displayed to the user on import. This will allow Alerting to import saved-objects with missing attributes, and warn the user that they must perform addition actions before these alerts are active.

See #82086 for the original discussion.

[elasticmachine]

Pinging @elastic/kibana-core (Team:Core)

[pgayvallet]

Note: this is probably tightly coupled to #84980, as warning propagation would probably use the same API/hooks as the custom export logic.

[rudolf]

I guess we need to decide what it means to "show a warning". Will a plugin just register a string that Core renders, or will we accept html (react components) or even a callback that can execute public code.

A string might be sufficient as a start, but it feels like being able to format the message and navigate to the app so a user can take the appropriate actions would be worth it.

Since this relates to the UI, I think this should be a Public (browser) API.

[pgayvallet]

guess we need to decide what it means to "show a warning". Will a plugin just register a string that Core renders, or will we accept html (react components) or even a callback that can execute public code

These 'warnings' will (edit: well, imho 😅 ) be generated from the server-side, so I think that's just deal breaker to think about arbitrary rendering. If we need to support warnings more complex than plain text, I think we would have to provide a correct data structure/format for it, and handle the rendering ourselves on the client-side from SO management.

For example, the warning type could look like:

type BaseWarning {
   type: string;
}

type SimpleWarning extends BaseWarning {
   type: 'simple';
   message: string;
}

type RedirectWarning extends BaseWarning {
   type: 'redirect';
   message: string;
   linkUrl: string;
   linkText: string;
}
type SoImportWarning = SimpleWarning | RedirectWarning;

and an example of RedirectWarning would be:

{
   message: 'Alert XXX was imported but need to be enabled',
   linkUrl: '/app/alerting/alert/XXX',
   linkText: 'Go here to enable this alert',
}

Since this relates to the UI, I think this should be a Public (browser) API.

I would really like to avoid the SO export/import logic leaking to the client side to be honest. Also, the import file is currently only parsed/read on the server-side, which mean we would need to return the full list of imported object (and their data) to the client for consumers to generate their warnings on the client-side. What upsides do you see in having this on the client-side?

[joshdover]

@mdefazio I believe you've done the recent design work on the Saved Object import flyout UI. We'd like to add a UI that shows warnings about additional actions that a user may need to take when importing objects. I was hoping you could help us figure out how we should display this.

Basic requirements:

• Warning messages are not tied to any specific object. A single warning message may be presented for a group of objects of a specific type.
• We need to be able to display warnings that are just text, as well as a message with text and a link / call to action to go to another location in Kibana to resolve the issue.

The one key example of a warning that users may see (please excuse my terrible copywriting):

• "Alerts will not be active until connector credentials are re-entered"

[pgayvallet]

@mikecote @kobelb

During the breakout session, we discussed about the expected warning(s) we should display when importing alerts.

The main question was: do we ideally want to have one warning per alert, or should we only have one simple warning when at least one alert requiring a post-import action was imported.

E.g, when importing 2 alerts, do we want to display

- Alert XXX will not be active until [stuff] is done. Go [here] to fix.
- Alert YYY will not be active until [stuff] is done. Go [here] to fix.

or

- Alerts will not be active until [stuff] is done. Go [here] to fix.

Also, @kobelb, apart from alerts, are we already aware of any type(s) that would currently need to leverage this import warning feature?

[kobelb]

Also, @kobelb, apart from alerts, are we already aware of any type(s) that would currently need to leverage this import warning feature?

At this moment, I'm only aware of alerts and actions needing this functionality.

[mikecote]

The main question was: do we ideally want to have one warning per alert, or should we only have one simple warning when at least one alert requiring a post-import action was imported.

In my opinion, I think it would be best to have a single message and ideally the "here" link can navigate to a filtered list of those imported alerts (something on our end to figure out). It may be a common scenario to import over 300 alerts (ex: SIEM detection rules).

cc @elastic/kibana-alerting-services for awareness.

[mdefazio]

Sorry for coming in late on this. If I'm understanding the requirements, it sounds like Mike's suggestion above makes the most sense (and we've already designed out this flow).

However, correct me if I'm wrong, but this would simply filter to alerts with errors, not specific to newly imported warnings. I think that's still ok—just checking. I'm wondering if we should add a message during import that forewarns the user that additional actions may need to be taken?

[pgayvallet]

@mdefazio

and we've already designed out this flow

Do you mind sharing where?

but this would simply filter to alerts with errors, not specific to newly imported warnings. I think that's still ok—just checking

That would be the easiest yes. However it would always be possible to pass the list of alerts to display in the warning's url, like linkUrl: '/app/alerting?ids=...',

[mdefazio]

@pgayvallet , after discussing further with Mike, I realized that I was a little mistaken about the requirements for this. My comment about the flow that was already in place was in regards to filtering the alert list based on alerts with errors. However, we do not have a flow to direct them there after importing.

[pgayvallet]

I started a POC of the implementation in #87996. It should be rather straightforward.

My main interrogation here is about the design. This is the current import modal when the import succeeds:

I'm unsure where / how exactly we should be adding the warnings. Using notifications doesn't seems like a good idea, as it would cause them to fade at some point, so I guess we need to have them displayed somewhere inside the import flyout.

@joshdover @alexfrancoeur do you know how we could progress on that? Should I ping design, or is there already someone from design assigned to this area?

[mdefazio]

@pgayvallet , I apologize for the delay in getting this moving. Perhaps its worth jumping on a quick call and we can sketch out where these warnings can live so you're not held up any longer

[mdefazio]

Posting here after our quick zoom chat:
We will show another view in the flyout if an Alert (or potentially other object types) is imported that tells the user there are possibly additional actions that need to be taken (and Go to Alerts to fix). Once in Alerts, we can show the error banner on the main list page.
It is possible that the user will simply close the flyout with the 'X' in which case they won't see that additional screen. So if we find that this happens more frequently, we discussed adding a small warning callout below the summary count ('47 overwritten') that hints that there are additional actions to be taken in the case of alerts. Clicking on this, would then change the view to the one mentioned above.

[mdefazio]

After chatting with @mikecote , let's go with the following option, where we place a warning on the summary page. This obviously needs some copy help. But this way, we can avoid the scenario of the user closing the flyout without seeing the warning message.