[drilldowns] do not add ON_APPLY_FILTER to embeddable supported triggers#261018
[drilldowns] do not add ON_APPLY_FILTER to embeddable supported triggers#261018nreese merged 4 commits intoelastic:mainfrom
Conversation
|
/ci |
|
/ci |
⏳ Build in-progress, with failures
Failed CI StepsHistory
|
|
/ci |
| * or ON_SELECT_RANGE is supported. | ||
| * @param triggers | ||
| */ | ||
| function ensureNestedTriggers(triggers: string[]): string[] { |
There was a problem hiding this comment.
So now lens specifically adds these triggers clientside? I'm not sure why we need to keep this function and move it into Lens rather than changing Lens to include the ON_APPLY_FILTER trigger
There was a problem hiding this comment.
Lens defines triggers for each vis type in services.visualizationMap. I chatted with @nickofthyme about this, and we decided the cleanest implementation was to add ON_APPLY_FILTER to getSupportedTriggers.
There was a problem hiding this comment.
If you want you could create an issue for us to investigate why this was done in the first place and clean it up later.
There was a problem hiding this comment.
A followup issue would be great yes
ThomThomson
left a comment
There was a problem hiding this comment.
Changes LGTM! Code only review
|
Starting backport for target branches: 9.5 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
|
Main is still tracking 9.4 so no backport needed |
Follow up to #259637
Before this PR, drilldown UI code added ON_APPLY_FILTER to an embeddable's supported triggers. This auto-magically allowed embeddables that implemented HasSupportedTriggers interface to support drilldowns that used ON_APPLY_FILTER trigger.
This auto-magic could cause problems with "as code" schemas. Embeddables register supported triggers on server during schema generation. Embeddables could not list ON_APPLY_FILTER in their supported triggers, allowing the UI to create drilldowns with triggers that are not contained in the schema. In practice this did not happen, but we want to avoid the possibility for such a mistake.
This PR resolves the issue by removing the magic. Drilldown UI code no longer adds ON_APPLY_FILTER
to an embeddable's supported triggers. Instead, embeddables must explicitly includeON_APPLY_FILTER in supported triggers list.