[Dashboard] [Controls] Prevent control group reload on output change (#154763)

Closes https://github.com/elastic/kibana/issues/154146

## Summary

This PR fixes a race condition for chained options list controls where
making changes to the output of the first control in the chain would
sometimes cause the chained controls to be stuck in an infinite loading
state. (Thanks to @ThomThomson for helping me narrow this down).

Basically, before this, **any** change to the control group output (for
example, by making a selection in an options list control) would cause
the dashboard to `forceRefresh` the entire control group:


682e2ed6ae/src/plugins/dashboard/public/dashboard_container/embeddable/integrations/controls/dashboard_control_group_integration.ts (L174-L185)

So, imagine you have a dashboard with two chained controls: control A
and control B. Making a selection in control A will cause the following
chain of events:
1. Make a selection in control A
2. Control B refetches its suggestions because hierarchical chaining is
turned on
3. At "the same time" (more-or-less), the subscription above fires due
to step 1 changing the control group output, and so the dashboard forces
a refresh of the control group
4. This causes both control A and control B to refetch their suggestions
unnecessarily, due to the `reload` logic of
`options_list_embeddable.tsx`.


682e2ed6ae/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx (L417-L421)

Because control B has now had two of the "same" requests sent to the
server, this caused a race condition where, depending on how fast things
completed and the state of the caching (which is cleared on `reload` but
is **not** cleared when a new selection is made), sometimes **both**
requests would get aborted and so the following early return would
prevent control B from setting its loading state to `false` (i.e. it
would get stuck in an infinite loading state):


682e2ed6ae/src/plugins/controls/public/options_list/embeddable/options_list_embeddable.tsx (L329-L337)


This PR prevents this race condition by only refreshing the
**dashboard's children** (i.e. the visualization embeddables) and not
the control group itself when the control group's output changes - as an
extra benefit, this makes the control group more efficient than it was
before since the unnecessary refresh is no longer slowing things down 💃

### Flaky Test Runner

<a
href="https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2114"><img
src="https://user-images.githubusercontent.com/8698078/231301760-a0c16e9b-fa7e-426c-9483-077527d39faa.png"/></a>

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

src/plugins/controls/public/options_list/components/options_list_popover_footer.tsx

@@ -54,7 +54,11 @@ export const OptionsListPopoverFooter = ({ isLoading }: { isLoading: boolean })
       >
         {isLoading && (
           <div style={{ position: 'absolute', width: '100%' }}>
-            <EuiProgress size="xs" color="accent" />
+            <EuiProgress
+              data-test-subj="optionsList-control-popover-loading"
+              size="xs"
+              color="accent"
+            />
           </div>
         )}
         <div

src/plugins/dashboard/public/dashboard_container/embeddable/dashboard_container.tsx

@@ -528,13 +528,13 @@ export class DashboardContainer extends Container<InheritedChildInput, Dashboard
   public showPlaceholderUntil = showPlaceholderUntil;
   public addOrUpdateEmbeddable = addOrUpdateEmbeddable;
 
-  public forceRefresh() {
+  public forceRefresh(refreshControlGroup: boolean = true) {
     const {
       dispatch,
       actions: { setLastReloadRequestTimeToNow },
     } = this.getReduxEmbeddableTools();
     dispatch(setLastReloadRequestTimeToNow({}));
-    this.controlGroup?.reload();
+    if (refreshControlGroup) this.controlGroup?.reload();
   }
 
   public onDataViewsUpdate$ = new Subject<DataView[]>();

src/plugins/dashboard/public/dashboard_container/embeddable/integrations/controls/dashboard_control_group_integration.ts

@@ -181,7 +181,7 @@ function startSyncingDashboardControlGroup(this: DashboardContainer) {
         ),
         skip(1) // skip first filter output because it will have been applied in initialize
       )
-      .subscribe(() => this.forceRefresh())
+      .subscribe(() => this.forceRefresh(false)) // we should not reload the control group when the control group output changes - otherwise, performance is severely impacted
   );
 
   subscriptions.add(

test/functional/apps/dashboard_elements/controls/control_group_chaining.ts

@@ -26,8 +26,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
     'header',
   ]);
 
-  // FLAKY: https://github.com/elastic/kibana/issues/154146
-  describe.skip('Dashboard control group hierarchical chaining', () => {
+  describe('Dashboard control group hierarchical chaining', () => {
     const newDocuments: Array<{ index: string; id: string }> = [];
     let controlIds: string[];
 
@@ -44,7 +43,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
 
       /* start by adding some incomplete data so that we can test `exists` query */
       await common.navigateToApp('console');
-      await console.collapseHelp();
+      await console.closeHelpIfExists();
       await console.clearTextArea();
       await addDocument(
         'animals-cats-2018-01-01',
@@ -88,7 +87,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
 
     after(async () => {
       await common.navigateToApp('console');
-      await console.collapseHelp();
+      await console.closeHelpIfExists();
       await console.clearTextArea();
       for (const { index, id } of newDocuments) {
         await console.enterRequest(`\nDELETE /${index}/_doc/${id}`);

test/functional/page_objects/dashboard_page_controls.ts

@@ -448,12 +448,14 @@ export class DashboardPageControls extends FtrService {
     this.log.debug(`searching for ${search} in options list`);
     await this.optionsListPopoverAssertOpen();
     await this.testSubjects.setValue(`optionsList-control-search-input`, search);
+    await this.optionsListPopoverWaitForLoading();
   }
 
   public async optionsListPopoverClearSearch() {
     this.log.debug(`clearing search from options list`);
     await this.optionsListPopoverAssertOpen();
     await this.find.clickByCssSelector('.euiFormControlLayoutClearButton');
+    await this.optionsListPopoverWaitForLoading();
   }
 
   public async optionsListPopoverSetSort(sort: OptionsListSortingType) {
@@ -464,14 +466,14 @@ export class DashboardPageControls extends FtrService {
     await this.retry.try(async () => {
       await this.testSubjects.existOrFail('optionsListControl__sortingOptionsPopover');
     });
-
     await this.testSubjects.click(`optionsList__sortOrder_${sort.direction}`);
     await this.testSubjects.click(`optionsList__sortBy_${sort.by}`);
-
     await this.testSubjects.click('optionsListControl__sortingOptionsButton');
     await this.retry.try(async () => {
       await this.testSubjects.missingOrFail(`optionsListControl__sortingOptionsPopover`);
     });
+
+    await this.optionsListPopoverWaitForLoading();
   }
 
   public async optionsListPopoverSelectExists() {
@@ -484,7 +486,6 @@ export class DashboardPageControls extends FtrService {
   public async optionsListPopoverSelectOption(availableOption: string) {
     this.log.debug(`selecting ${availableOption} from options list`);
     await this.optionsListPopoverSearchForOption(availableOption);
-    await this.optionsListPopoverWaitForLoading();
 
     await this.retry.try(async () => {
       await this.testSubjects.existOrFail(`optionsList-control-selection-${availableOption}`);
@@ -492,7 +493,6 @@ export class DashboardPageControls extends FtrService {
     });
 
     await this.optionsListPopoverClearSearch();
-    await this.optionsListPopoverWaitForLoading();
   }
 
   public async optionsListPopoverClearSelections() {
@@ -516,7 +516,10 @@ export class DashboardPageControls extends FtrService {
 
   public async optionsListWaitForLoading(controlId: string) {
     this.log.debug(`wait for ${controlId} to load`);
-    await this.testSubjects.waitForEnabled(`optionsList-control-${controlId}`);
+    const enabled = await this.testSubjects.waitForEnabled(`optionsList-control-${controlId}`);
+    if (!enabled) {
+      throw new Error(`${controlId} did not finish loading within the given time limit`);
+    }
   }
 
   public async optionsListPopoverWaitForLoading() {

test/functional/services/common/test_subjects.ts

@@ -360,11 +360,12 @@ export class TestSubjects extends FtrService {
     await this.findService.waitForElementHidden(element, timeout);
   }
 
-  public async waitForEnabled(selector: string, timeout: number = this.TRY_TIME): Promise<void> {
-    await this.retry.tryForTime(timeout, async () => {
+  public async waitForEnabled(selector: string, timeout: number = this.TRY_TIME): Promise<boolean> {
+    const success = await this.retry.tryForTime(timeout, async () => {
       const element = await this.find(selector);
       return (await element.isDisplayed()) && (await element.isEnabled());
     });
+    return success;
   }
 
   public getCssSelector(selector: string): string {