[Guided onboarding] Fix multiple API requests when fetching guide state

packages/kbn-guided-onboarding/index.ts

@@ -6,6 +6,13 @@
  * Side Public License, v 1.
  */
 
-export type { GuideState, GuideId, GuideStepIds, StepStatus, GuideStep } from './src/types';
+export type {
+  GuideState,
+  GuideId,
+  GuideStepIds,
+  StepStatus,
+  GuideStep,
+  GuideStatus,
+} from './src/types';
 export { GuideCard, ObservabilityLinkCard } from './src/components/landing_page';
 export type { UseCase } from './src/components/landing_page';

src/plugins/guided_onboarding/public/components/guide_panel.test.tsx

@@ -109,10 +109,21 @@ describe('Guided setup', () => {
   });
 
   describe('Button component', () => {
-    // TODO check for the correct button behavior once https://github.com/elastic/kibana/issues/141129 is implemented
-    test.skip('should be disabled in there is no active guide', async () => {
+    test('should be hidden in there is no guide state', async () => {
       const { exists } = testBed;
-      expect(exists('disabledGuideButton')).toBe(true);
+      expect(exists('guideButton')).toBe(false);
+      expect(exists('guidePanel')).toBe(false);
+    });
+
+    test('should be hidden if the guide is not active', async () => {
+      const { component, exists } = testBed;
+
+      await updateComponentWithState(
+        component,
+        { ...mockActiveSearchGuideState, isActive: false },
+        true
+      );
+
       expect(exists('guideButton')).toBe(false);
       expect(exists('guidePanel')).toBe(false);
     });

src/plugins/guided_onboarding/public/components/guide_panel.tsx

@@ -139,7 +139,7 @@ export const GuidePanel = ({ api, application }: GuidePanelProps) => {
 
   // TODO handle loading, error state
   // https://github.com/elastic/kibana/issues/139799, https://github.com/elastic/kibana/issues/139798
-  if (!guideConfig) {
+  if (!guideConfig || !guideState || !guideState.isActive) {
     // TODO button show/hide logic https://github.com/elastic/kibana/issues/141129
     return null;
   }

src/plugins/guided_onboarding/public/services/api.mocks.ts

@@ -77,6 +77,24 @@ export const testGuideStep2InProgressState: GuideState = {
   ],
 };
 
+export const readyToCompleteGuideState: GuideState = {
+  ...testGuideStep1ActiveState,
+  steps: [
+    {
+      ...testGuideStep1ActiveState.steps[0],
+      status: 'complete',
+    },
+    {
+      ...testGuideStep1ActiveState.steps[1],
+      status: 'complete',
+    },
+    {
+      ...testGuideStep1ActiveState.steps[2],
+      status: 'complete',
+    },
+  ],
+};
+
 export const testGuideNotActiveState: GuideState = {
   ...testGuideStep1ActiveState,
   isActive: false,

src/plugins/guided_onboarding/public/services/api.test.ts

@@ -11,6 +11,7 @@ import { httpServiceMock } from '@kbn/core/public/mocks';
 import type { GuideState } from '@kbn/guided-onboarding';
 import { firstValueFrom, Subscription } from 'rxjs';
 
+import { GuideStatus } from '@kbn/guided-onboarding';
 import { API_BASE_PATH } from '../../common/constants';
 import { ApiService } from './api';
 import {
@@ -24,12 +25,14 @@ import {
   testIntegration,
   wrongIntegration,
   testGuideStep2InProgressState,
+  readyToCompleteGuideState,
 } from './api.mocks';
 
 describe('GuidedOnboarding ApiService', () => {
   let httpClient: jest.Mocked<HttpSetup>;
   let apiService: ApiService;
   let subscription: Subscription;
+  let anotherSubscription: Subscription;
 
   beforeEach(() => {
     httpClient = httpServiceMock.createStartContract({ basePath: '/base/path' });
@@ -41,9 +44,8 @@ describe('GuidedOnboarding ApiService', () => {
   });
 
   afterEach(() => {
-    if (subscription) {
-      subscription.unsubscribe();
-    }
+    subscription?.unsubscribe();
+    anotherSubscription?.unsubscribe();
     jest.restoreAllMocks();
   });
 
@@ -53,6 +55,64 @@ describe('GuidedOnboarding ApiService', () => {
       expect(httpClient.get).toHaveBeenCalledTimes(1);
       expect(httpClient.get).toHaveBeenCalledWith(`${API_BASE_PATH}/state`, {
         query: { active: true },
+        signal: new AbortController().signal,
+      });
+    });
+
+    it(`doesn't send multiple requests when there are several subscriptions`, () => {
+      subscription = apiService.fetchActiveGuideState$().subscribe();
+      anotherSubscription = apiService.fetchActiveGuideState$().subscribe();
+      expect(httpClient.get).toHaveBeenCalledTimes(1);
+    });
+
+    it(`re-sends the request if the previous one failed`, async () => {
+      httpClient.get.mockRejectedValueOnce(new Error('request failed'));
+      subscription = apiService.fetchActiveGuideState$().subscribe();
+      // wait until the request fails
+      await new Promise((resolve) => process.nextTick(resolve));
+      anotherSubscription = apiService.fetchActiveGuideState$().subscribe();
+      expect(httpClient.get).toHaveBeenCalledTimes(2);
+    });
+
+    it(`re-sends the request if there is no guide state and there is another subscription`, async () => {
+      httpClient.get.mockResolvedValueOnce({
+        state: [],
+      });
+      subscription = apiService.fetchActiveGuideState$().subscribe();
+      // wait until the request completes
+      await new Promise((resolve) => process.nextTick(resolve));
+      anotherSubscription = apiService.fetchActiveGuideState$().subscribe();
+      expect(httpClient.get).toHaveBeenCalledTimes(2);
+    });
+
+    it(`doesn't send multiple requests in a loop when there is no state`, async () => {
+      httpClient.get.mockResolvedValueOnce({
+        state: [],
+      });
+      subscription = apiService.fetchActiveGuideState$().subscribe();
+      // wait until the request completes
+      await new Promise((resolve) => process.nextTick(resolve));
+      expect(httpClient.get).toHaveBeenCalledTimes(1);
+    });
+
+    it(`re-sends the request if the subscription was unsubscribed before the request completed`, async () => {
+      httpClient.get.mockImplementationOnce(() => {
+        return new Promise((resolve) => setTimeout(resolve));
+      });
+      // subscribe and immediately unsubscribe
+      apiService.fetchActiveGuideState$().subscribe().unsubscribe();
+      anotherSubscription = apiService.fetchActiveGuideState$().subscribe();
+      expect(httpClient.get).toHaveBeenCalledTimes(2);
+    });
+
+    it(`the second subscription gets the state broadcast to it`, (done) => {
+      // first subscription
+      apiService.fetchActiveGuideState$().subscribe();
+      // second subscription
+      anotherSubscription = apiService.fetchActiveGuideState$().subscribe((state) => {
+        if (state) {
+          done();
+        }
       });
     });
 
@@ -95,6 +155,17 @@ describe('GuidedOnboarding ApiService', () => {
         body: JSON.stringify(updatedState),
       });
     });
+
+    it('the completed state is being broadcast after the update', async () => {
+      const completedState = {
+        ...readyToCompleteGuideState,
+        isActive: false,
+        status: 'complete' as GuideStatus,
+      };
+      await apiService.updateGuideState(completedState, false);
+      const state = await firstValueFrom(apiService.fetchActiveGuideState$());
+      expect(state).toMatchObject(completedState);
+    });
   });
 
   describe('isGuideStepActive$', () => {
@@ -149,24 +220,6 @@ describe('GuidedOnboarding ApiService', () => {
   });
 
   describe('completeGuide', () => {
-    const readyToCompleteGuideState: GuideState = {
-      ...testGuideStep1ActiveState,
-      steps: [
-        {
-          ...testGuideStep1ActiveState.steps[0],
-          status: 'complete',
-        },
-        {
-          ...testGuideStep1ActiveState.steps[1],
-          status: 'complete',
-        },
-        {
-          ...testGuideStep1ActiveState.steps[2],
-          status: 'complete',
-        },
-      ],
-    };
-
     beforeEach(async () => {
       await apiService.updateGuideState(readyToCompleteGuideState, false);
     });

src/plugins/guided_onboarding/public/services/api.ts

@@ -7,7 +7,7 @@
  */
 
 import { HttpSetup } from '@kbn/core/public';
-import { BehaviorSubject, map, from, concatMap, of, Observable, firstValueFrom } from 'rxjs';
+import { BehaviorSubject, map, concatMap, of, Observable, firstValueFrom } from 'rxjs';
 import type { GuideState, GuideId, GuideStep, GuideStepIds } from '@kbn/guided-onboarding';
 
 import { GuidedOnboardingApi } from '../types';
@@ -26,37 +26,54 @@ import { API_BASE_PATH } from '../../common/constants';
 export class ApiService implements GuidedOnboardingApi {
   private client: HttpSetup | undefined;
   private onboardingGuideState$!: BehaviorSubject<GuideState | undefined>;
+  private isGuideStateLoading: boolean | undefined;
   public isGuidePanelOpen$: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(false);
 
   public setup(httpClient: HttpSetup): void {
     this.client = httpClient;
     this.onboardingGuideState$ = new BehaviorSubject<GuideState | undefined>(undefined);
   }
 
+  private createGetStateObservable(): Observable<GuideState | undefined> {
+    return new Observable<GuideState | undefined>((observer) => {
+      const controller = new AbortController();
+      const signal = controller.signal;
+      this.isGuideStateLoading = true;
+      this.client!.get<{ state: GuideState[] }>(`${API_BASE_PATH}/state`, {
+        query: {
+          active: true,
+        },
+        signal,
+      })
+        .then((response) => {
+          this.isGuideStateLoading = false;
+          // There should only be 1 active guide
+          const hasState = response.state.length === 1;
+          if (hasState) {
+            this.onboardingGuideState$.next(response.state[0]);
+          }
+          observer.complete();
+        })
+        .catch((error) => {
+          this.isGuideStateLoading = false;
+          observer.error(error);
+        });
+      return () => {
+        this.isGuideStateLoading = false;
+        controller.abort();
+      };
+    });
+  }
+
   /**
    * An Observable with the active guide state.
    * Initially the state is fetched from the backend.
    * Subsequently, the observable is updated automatically, when the state changes.
    */
   public fetchActiveGuideState$(): Observable<GuideState | undefined> {
-    // TODO add error handling if this.client has not been initialized or request fails
     return this.onboardingGuideState$.pipe(
       concatMap((state) =>
-        state === undefined
-          ? from(
-              this.client!.get<{ state: GuideState[] }>(`${API_BASE_PATH}/state`, {
-                query: {
-                  active: true,
-                },
-              })
-            ).pipe(
-              map((response) => {
-                // There should only be 1 active guide
-                const hasState = response.state.length === 1;
-                return hasState ? response.state[0] : undefined;
-              })
-            )
-          : of(state)
+        !state && !this.isGuideStateLoading ? this.createGetStateObservable() : of(state)
       )
     );
   }
@@ -83,7 +100,7 @@ export class ApiService implements GuidedOnboardingApi {
   /**
    * Updates the SO with the updated guide state and refreshes the observables
    * This is largely used internally and for tests
-   * @param {GuideState} guideState the updated guide state
+   * @param {GuideState} newState the updated guide state
    * @param {boolean} panelState boolean to determine whether the dropdown panel should open or not
    * @return {Promise} a promise with the updated guide state
    */
@@ -99,8 +116,8 @@ export class ApiService implements GuidedOnboardingApi {
       const response = await this.client.put<{ state: GuideState }>(`${API_BASE_PATH}/state`, {
         body: JSON.stringify(newState),
       });
-      // If the guide has been deactivated, we return undefined
-      this.onboardingGuideState$.next(newState.isActive ? newState : undefined);
+      // broadcast the newState
+      this.onboardingGuideState$.next(newState);
       this.isGuidePanelOpen$.next(panelState);
       return response;
     } catch (error) {