[Guided onboarding] More API fixes

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

@@ -74,15 +74,15 @@ describe('GuidedOnboarding ApiService', () => {
       expect(httpClient.get).toHaveBeenCalledTimes(2);
     });
 
-    it(`re-sends the request if there is no guide state and there is another subscription`, async () => {
+    it(`doesn't re-send 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);
+      expect(httpClient.get).toHaveBeenCalledTimes(1);
     });
 
     it(`doesn't send multiple requests in a loop when there is no state`, async () => {
@@ -170,8 +170,9 @@ describe('GuidedOnboarding ApiService', () => {
 
   describe('isGuideStepActive$', () => {
     it('returns true if the step has been started', (done) => {
-      const updatedState: GuideState = testGuideStep1InProgressState;
-      apiService.updateGuideState(updatedState, false);
+      httpClient.get.mockResolvedValueOnce({
+        state: [testGuideStep1InProgressState],
+      });
 
       subscription = apiService
         .isGuideStepActive$(testGuide, testGuideFirstStep)
@@ -191,6 +192,23 @@ describe('GuidedOnboarding ApiService', () => {
           }
         });
     });
+
+    it(`doesn't duplicate requests when there are several subscriptions and no guide state`, async () => {
+      httpClient.get.mockResolvedValue({
+        state: [],
+      });
+      apiService.setup(httpClient);
+
+      subscription = apiService.isGuideStepActive$(testGuide, testGuideFirstStep).subscribe();
+
+      // wait for the get request to resolve
+      await new Promise((resolve) => process.nextTick(resolve));
+      anotherSubscription = apiService
+        .isGuideStepActive$(testGuide, testGuideFirstStep)
+        .subscribe();
+
+      expect(httpClient.get).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('activateGuide', () => {
@@ -221,7 +239,10 @@ describe('GuidedOnboarding ApiService', () => {
 
   describe('completeGuide', () => {
     beforeEach(async () => {
-      await apiService.updateGuideState(readyToCompleteGuideState, false);
+      httpClient.get.mockResolvedValue({
+        state: [readyToCompleteGuideState],
+      });
+      apiService.setup(httpClient);
     });
 
     it('updates the selected guide and marks it as complete', async () => {
@@ -259,16 +280,21 @@ describe('GuidedOnboarding ApiService', () => {
           },
         ],
       };
-      await apiService.updateGuideState(incompleteGuideState, false);
-
+      httpClient.get.mockResolvedValue({
+        state: [incompleteGuideState],
+      });
+      apiService.setup(httpClient);
       const completedState = await apiService.completeGuide(testGuide);
       expect(completedState).not.toBeDefined();
     });
   });
 
   describe('startGuideStep', () => {
     beforeEach(async () => {
-      await apiService.updateGuideState(testGuideStep1ActiveState, false);
+      httpClient.get.mockResolvedValue({
+        state: [testGuideStep1ActiveState],
+      });
+      apiService.setup(httpClient);
     });
 
     it('updates the selected step and marks it as in_progress', async () => {
@@ -287,12 +313,14 @@ describe('GuidedOnboarding ApiService', () => {
 
   describe('completeGuideStep', () => {
     it(`completes the step when it's in progress`, async () => {
-      await apiService.updateGuideState(testGuideStep1InProgressState, false);
+      httpClient.get.mockResolvedValue({
+        state: [testGuideStep1InProgressState],
+      });
+      apiService.setup(httpClient);
 
       await apiService.completeGuideStep(testGuide, testGuideFirstStep);
 
-      // Once on update, once on complete
-      expect(httpClient.put).toHaveBeenCalledTimes(2);
+      expect(httpClient.put).toHaveBeenCalledTimes(1);
       // Verify the completed step now has a "complete" status, and the subsequent step is "active"
       expect(httpClient.put).toHaveBeenLastCalledWith(`${API_BASE_PATH}/state`, {
         body: JSON.stringify({ ...testGuideStep2ActiveState }),

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

@@ -7,7 +7,7 @@
  */
 
 import { HttpSetup } from '@kbn/core/public';
-import { BehaviorSubject, map, concatMap, of, Observable, firstValueFrom } from 'rxjs';
+import { BehaviorSubject, map, Observable, firstValueFrom, concat } from 'rxjs';
 import type { GuideState, GuideId, GuideStep, GuideStepIds } from '@kbn/guided-onboarding';
 
 import { GuidedOnboardingApi } from '../types';
@@ -25,13 +25,14 @@ import { API_BASE_PATH } from '../../common/constants';
 
 export class ApiService implements GuidedOnboardingApi {
   private client: HttpSetup | undefined;
-  private onboardingGuideState$!: BehaviorSubject<GuideState | undefined>;
+  private guideState$!: BehaviorSubject<GuideState | undefined>;
   private isGuideStateLoading: boolean | undefined;
+  private isGuideStateInitialized: 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);
+    this.guideState$ = new BehaviorSubject<GuideState | undefined>(undefined);
   }
 
   private createGetStateObservable(): Observable<GuideState | undefined> {
@@ -46,11 +47,13 @@ export class ApiService implements GuidedOnboardingApi {
         signal,
       })
         .then((response) => {
+          this.isGuideStateInitialized = true;
           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.next(response.state[0]);
+            this.guideState$.next(response.state[0]);
           }
           observer.complete();
         })
@@ -71,11 +74,15 @@ export class ApiService implements GuidedOnboardingApi {
    * Subsequently, the observable is updated automatically, when the state changes.
    */
   public fetchActiveGuideState$(): Observable<GuideState | undefined> {
-    return this.onboardingGuideState$.pipe(
-      concatMap((state) =>
-        !state && !this.isGuideStateLoading ? this.createGetStateObservable() : of(state)
-      )
-    );
+    const currentState = this.guideState$.value;
+    // if currentState is undefined, it can be because there is no active guide or we haven't fetched the data from the backend
+    // check if there is no request in flight
+    // also check if we have fetched the data from the backend already once, if yes no request is sent
+    if (!currentState && !this.isGuideStateLoading && !this.isGuideStateInitialized) {
+      this.isGuideStateLoading = true;
+      return concat(this.createGetStateObservable(), this.guideState$);
+    }
+    return this.guideState$;
   }
 
   /**
@@ -117,7 +124,7 @@ export class ApiService implements GuidedOnboardingApi {
         body: JSON.stringify(newState),
       });
       // broadcast the newState
-      this.onboardingGuideState$.next(newState);
+      this.guideState$.next(newState);
       this.isGuidePanelOpen$.next(panelState);
       return response;
     } catch (error) {