[8.16] [Discover] Show the fetched Discover results even when histogr…

…am request fails on some shards (#198553) (#199123) # Backport This will backport the following commits from `main` to `8.16`: - [[Discover] Show the fetched Discover results even when histogram request fails on some shards (#198553)](#198553)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Julia Rechkunova <[email protected]>
elastic · Nov 6, 2024 · 3a3d0c1 · 3a3d0c1
1 parent 1557905
commit 3a3d0c1
Show file tree

Hide file tree

Showing 6 changed files with 119 additions and 31 deletions.
diff --git a/src/plugins/discover/public/components/hits_counter/hits_counter.test.tsx b/src/plugins/discover/public/components/hits_counter/hits_counter.test.tsx
@@ -14,8 +14,20 @@ import { findTestSubject } from '@elastic/eui/lib/test';
 import { EuiLoadingSpinner } from '@elastic/eui';
 import { BehaviorSubject } from 'rxjs';
 import { getDiscoverStateMock } from '../../__mocks__/discover_state.mock';
-import { DataTotalHits$ } from '../../application/main/state_management/discover_data_state_container';
+import {
+  DataDocuments$,
+  DataTotalHits$,
+} from '../../application/main/state_management/discover_data_state_container';
 import { FetchStatus } from '../../application/types';
+import { dataViewMock, esHitsMock } from '@kbn/discover-utils/src/__mocks__';
+import { buildDataTableRecord } from '@kbn/discover-utils';
+
+function getDocuments$(count: number = 5) {
+  return new BehaviorSubject({
+    fetchStatus: FetchStatus.COMPLETE,
+    result: esHitsMock.map((esHit) => buildDataTableRecord(esHit, dataViewMock)).slice(0, count),
+  }) as DataDocuments$;
+}
 
 describe('hits counter', function () {
   it('expect to render the number of hits', function () {
@@ -24,6 +36,7 @@ describe('hits counter', function () {
       fetchStatus: FetchStatus.COMPLETE,
       result: 1,
     }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$();
     const component1 = mountWithIntl(
       <HitsCounter mode={HitsCounterMode.appended} stateContainer={stateContainer} />
     );
@@ -45,6 +58,7 @@ describe('hits counter', function () {
       fetchStatus: FetchStatus.COMPLETE,
       result: 1899,
     }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$();
     const component1 = mountWithIntl(
       <HitsCounter mode={HitsCounterMode.appended} stateContainer={stateContainer} />
     );
@@ -64,6 +78,7 @@ describe('hits counter', function () {
       fetchStatus: FetchStatus.PARTIAL,
       result: 2,
     }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$();
     const component = mountWithIntl(
       <HitsCounter mode={HitsCounterMode.standalone} stateContainer={stateContainer} />
     );
@@ -76,6 +91,7 @@ describe('hits counter', function () {
       fetchStatus: FetchStatus.PARTIAL,
       result: 2,
     }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$();
     const component = mountWithIntl(
       <HitsCounter mode={HitsCounterMode.standalone} stateContainer={stateContainer} />
     );
@@ -89,9 +105,51 @@ describe('hits counter', function () {
       fetchStatus: FetchStatus.LOADING,
       result: undefined,
     }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$();
     const component = mountWithIntl(
       <HitsCounter mode={HitsCounterMode.standalone} stateContainer={stateContainer} />
     );
     expect(component.isEmptyRender()).toBe(true);
   });
+
+  it('should render discoverQueryHitsPartial when status is error', () => {
+    const stateContainer = getDiscoverStateMock({ isTimeBased: true });
+    stateContainer.dataState.data$.totalHits$ = new BehaviorSubject({
+      fetchStatus: FetchStatus.ERROR,
+      result: undefined,
+    }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$(3);
+    const component = mountWithIntl(
+      <HitsCounter mode={HitsCounterMode.standalone} stateContainer={stateContainer} />
+    );
+    expect(component.find('[data-test-subj="discoverQueryHitsPartial"]').length).toBe(1);
+    expect(findTestSubject(component, 'discoverQueryTotalHits').text()).toBe('≥3 resultsInfo');
+    expect(component.text()).toBe('≥3 resultsInfo');
+
+    stateContainer.dataState.data$.totalHits$ = new BehaviorSubject({
+      fetchStatus: FetchStatus.ERROR,
+      result: 200,
+    }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$(2);
+
+    const component2 = mountWithIntl(
+      <HitsCounter mode={HitsCounterMode.appended} stateContainer={stateContainer} />
+    );
+    expect(component2.find('[data-test-subj="discoverQueryHitsPartial"]').length).toBe(1);
+    expect(findTestSubject(component2, 'discoverQueryTotalHits').text()).toBe('≥200Info');
+    expect(component2.text()).toBe(' (≥200Info)');
+
+    stateContainer.dataState.data$.totalHits$ = new BehaviorSubject({
+      fetchStatus: FetchStatus.ERROR,
+      result: 0,
+    }) as DataTotalHits$;
+    stateContainer.dataState.data$.documents$ = getDocuments$(1);
+
+    const component3 = mountWithIntl(
+      <HitsCounter mode={HitsCounterMode.appended} stateContainer={stateContainer} />
+    );
+    expect(component3.find('[data-test-subj="discoverQueryHitsPartial"]').length).toBe(1);
+    expect(findTestSubject(component3, 'discoverQueryTotalHits').text()).toBe('≥1Info');
+    expect(component3.text()).toBe(' (≥1Info)');
+  });
 });
diff --git a/src/plugins/discover/public/components/hits_counter/hits_counter.tsx b/src/plugins/discover/public/components/hits_counter/hits_counter.tsx
@@ -8,7 +8,7 @@
  */
 
 import React from 'react';
-import { EuiFlexGroup, EuiFlexItem, EuiText, EuiLoadingSpinner } from '@elastic/eui';
+import { EuiFlexGroup, EuiFlexItem, EuiText, EuiLoadingSpinner, EuiIconTip } from '@elastic/eui';
 import { FormattedMessage, FormattedNumber } from '@kbn/i18n-react';
 import { i18n } from '@kbn/i18n';
 import { css } from '@emotion/react';
@@ -29,18 +29,33 @@ export interface HitsCounterProps {
 export const HitsCounter: React.FC<HitsCounterProps> = ({ mode, stateContainer }) => {
   const totalHits$ = stateContainer.dataState.data$.totalHits$;
   const totalHitsState = useDataState(totalHits$);
-  const hitsTotal = totalHitsState.result;
+  let hitsTotal = totalHitsState.result;
   const hitsStatus = totalHitsState.fetchStatus;
 
+  const documents$ = stateContainer.dataState.data$.documents$;
+  const documentsState = useDataState(documents$);
+  const documentsCount = documentsState.result?.length || 0;
+
   if (!hitsTotal && hitsStatus === FetchStatus.LOADING) {
     return null;
   }
 
+  if (
+    hitsStatus === FetchStatus.ERROR &&
+    documentsState.fetchStatus === FetchStatus.COMPLETE &&
+    documentsCount > (hitsTotal ?? 0)
+  ) {
+    // if histogram returned partial results and which are less than the fetched documents count =>
+    // override hitsTotal with the fetched documents count
+    hitsTotal = documentsCount;
+  }
+
+  const showGreaterOrEqualSign =
+    hitsStatus === FetchStatus.PARTIAL || hitsStatus === FetchStatus.ERROR;
+
   const formattedHits = (
     <span
-      data-test-subj={
-        hitsStatus === FetchStatus.PARTIAL ? 'discoverQueryHitsPartial' : 'discoverQueryHits'
-      }
+      data-test-subj={showGreaterOrEqualSign ? 'discoverQueryHitsPartial' : 'discoverQueryHits'}
     >
       <FormattedNumber value={hitsTotal ?? 0} />
     </span>
@@ -55,7 +70,7 @@ export const HitsCounter: React.FC<HitsCounterProps> = ({ mode, stateContainer }
 
   const element = (
     <EuiFlexGroup
-      gutterSize="s"
+      gutterSize="xs"
       responsive={false}
       justifyContent="center"
       alignItems="center"
@@ -66,8 +81,8 @@ export const HitsCounter: React.FC<HitsCounterProps> = ({ mode, stateContainer }
       <EuiFlexItem grow={false} aria-live="polite" css={hitsCounterTextCss}>
         <EuiText className="eui-textTruncate" size="s">
           <strong>
-            {hitsStatus === FetchStatus.PARTIAL &&
-              (mode === HitsCounterMode.standalone ? (
+            {showGreaterOrEqualSign ? (
+              mode === HitsCounterMode.standalone ? (
                 <FormattedMessage
                   id="discover.hitsCounter.partialHitsPluralTitle"
                   defaultMessage="≥{formattedHits} {hits, plural, one {result} other {results}}"
@@ -79,17 +94,16 @@ export const HitsCounter: React.FC<HitsCounterProps> = ({ mode, stateContainer }
                   defaultMessage="≥{formattedHits}"
                   values={{ formattedHits }}
                 />
-              ))}
-            {hitsStatus !== FetchStatus.PARTIAL &&
-              (mode === HitsCounterMode.standalone ? (
-                <FormattedMessage
-                  id="discover.hitsCounter.hitsPluralTitle"
-                  defaultMessage="{formattedHits} {hits, plural, one {result} other {results}}"
-                  values={{ hits: hitsTotal, formattedHits }}
-                />
-              ) : (
-                formattedHits
-              ))}
+              )
+            ) : mode === HitsCounterMode.standalone ? (
+              <FormattedMessage
+                id="discover.hitsCounter.hitsPluralTitle"
+                defaultMessage="{formattedHits} {hits, plural, one {result} other {results}}"
+                values={{ hits: hitsTotal, formattedHits }}
+              />
+            ) : (
+              formattedHits
+            )}
           </strong>
         </EuiText>
       </EuiFlexItem>
@@ -103,6 +117,19 @@ export const HitsCounter: React.FC<HitsCounterProps> = ({ mode, stateContainer }
           />
         </EuiFlexItem>
       )}
+      {hitsStatus === FetchStatus.ERROR && (
+        <EuiFlexItem grow={false}>
+          <EuiIconTip
+            type="warning"
+            color="warning"
+            size="s"
+            content={i18n.translate('discover.hitsCounter.hitCountWarningTooltip', {
+              defaultMessage: 'Results might be incomplete',
+            })}
+            iconProps={{ css: { display: 'block' } }}
+          />
+        </EuiFlexItem>
+      )}
     </EuiFlexGroup>
   );
 

diff --git a/src/plugins/unified_histogram/public/chart/histogram.test.tsx b/src/plugins/unified_histogram/public/chart/histogram.test.tsx
@@ -240,7 +240,7 @@ describe('Histogram', () => {
       onLoad(false, adapters);
     });
     expect(props.onTotalHitsChange).toHaveBeenLastCalledWith(
-      UnifiedHistogramFetchStatus.complete,
+      UnifiedHistogramFetchStatus.error,
       100
     );
     expect(props.onChartLoad).toHaveBeenLastCalledWith({ adapters });

diff --git a/src/plugins/unified_histogram/public/chart/histogram.tsx b/src/plugins/unified_histogram/public/chart/histogram.tsx
@@ -130,9 +130,6 @@ export function Histogram({
         | undefined;
       const response = json?.rawResponse;
 
-      // The response can have `response?._shards.failed` but we should still be able to show hits number
-      // TODO: show shards warnings as a badge next to the total hits number
-
       if (requestFailed) {
         onTotalHitsChange?.(UnifiedHistogramFetchStatus.error, undefined);
         onChartLoad?.({ adapters: adapters ?? {} });
@@ -142,10 +139,14 @@ export function Histogram({
       const adapterTables = adapters?.tables?.tables;
       const totalHits = computeTotalHits(hasLensSuggestions, adapterTables, isPlainRecord);
 
-      onTotalHitsChange?.(
-        isLoading ? UnifiedHistogramFetchStatus.loading : UnifiedHistogramFetchStatus.complete,
-        totalHits ?? hits?.total
-      );
+      if (response?._shards?.failed || response?.timed_out) {
+        onTotalHitsChange?.(UnifiedHistogramFetchStatus.error, totalHits);
+      } else {
+        onTotalHitsChange?.(
+          isLoading ? UnifiedHistogramFetchStatus.loading : UnifiedHistogramFetchStatus.complete,
+          totalHits ?? hits?.total
+        );
+      }
 
       if (response) {
         const newBucketInterval = buildBucketInterval({

diff --git a/test/functional/apps/discover/ccs_compatibility/_search_errors.ts b/test/functional/apps/discover/ccs_compatibility/_search_errors.ts
@@ -65,7 +65,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
 
       // Ensure documents are still returned for the successful shards
       await retry.try(async function tryingForTime() {
-        const hitCount = await discover.getHitCount();
+        const hitCount = await discover.getHitCount({ isPartial: true });
         expect(hitCount).to.be('9,247');
       });
 

diff --git a/test/functional/page_objects/discover_page.ts b/test/functional/page_objects/discover_page.ts
@@ -338,9 +338,11 @@ export class DiscoverPageObject extends FtrService {
     return await this.header.waitUntilLoadingHasFinished();
   }
 
-  public async getHitCount() {
+  public async getHitCount({ isPartial }: { isPartial?: boolean } = {}) {
     await this.header.waitUntilLoadingHasFinished();
-    return await this.testSubjects.getVisibleText('discoverQueryHits');
+    return await this.testSubjects.getVisibleText(
+      isPartial ? 'discoverQueryHitsPartial' : 'discoverQueryHits'
+    );
   }
 
   public async getHitCountInt() {