Revert "Revert "[Guided onboarding] Fix multiple API requests when fetching guide state (#144160)"" (#144459)

* Revert "Revert "[Guided onboarding] Fix multiple API requests when fetching guide state (#144160)""

This reverts commit a4445f959d.

* [Guided onboarding] Fix type errors
This commit is contained in:
Yulia Čech 2022-11-02 18:57:50 +01:00 committed by GitHub
parent 259caffdd2
commit 610f04a833
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 260 additions and 150 deletions

View file

@ -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';

View file

@ -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,
{ ...mockActiveTestGuideState, isActive: false },
true
);
expect(exists('guideButton')).toBe(false);
expect(exists('guidePanel')).toBe(false);
});
@ -123,7 +134,6 @@ describe('Guided setup', () => {
// Enable the "test" guide
await updateComponentWithState(component, mockActiveTestGuideState, true);
expect(exists('disabledGuideButton')).toBe(false);
expect(exists('guideButton')).toBe(true);
expect(find('guideButton').text()).toEqual('Setup guide');
});

View file

@ -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;
}

View file

@ -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,

View file

@ -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);
});

View file

@ -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,6 +26,7 @@ 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 {
@ -33,30 +34,46 @@ export class ApiService implements GuidedOnboardingApi {
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) {

View file

@ -15,6 +15,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
const pageObjects = getPageObjects(['common', 'infraHome']);
const find = getService('find');
const supertest = getService('supertest');
const deployment = getService('deployment');
const setInitialTourState = async (activeStep?: number) => {
await browser.setLocalStorageItem(observTourStepStorageKey, String(activeStep || 1));
@ -23,8 +24,9 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
describe('Onboarding Observability tour', function () {
this.tags('includeFirefox');
let isCloud: boolean;
before(async () => {
isCloud = await deployment.isCloud();
await esArchiver.load('x-pack/test/functional/es_archives/infra/metrics_and_logs');
await pageObjects.common.navigateToApp('observability');
// Need to increase the browser height so the tour steps fit to screen
@ -37,131 +39,134 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
});
describe('Tour enabled', () => {
beforeEach(async () => {
// Activate the Observability guide, step 3, in order to trigger the EuiTour
await supertest
.put(`/api/guided_onboarding/state`)
.set('kbn-xsrf', 'true')
.send({
status: 'in_progress',
guideId: 'observability',
isActive: true,
steps: [
{
id: 'add_data',
status: 'complete',
},
{
id: 'view_dashboard',
status: 'complete',
},
{
id: 'tour_observability',
status: 'in_progress',
},
],
})
.expect(200);
});
// only run these tests on Cloud
if (isCloud) {
beforeEach(async () => {
// Activate the Observability guide, step 3, in order to trigger the EuiTour
await supertest
.put(`/api/guided_onboarding/state`)
.set('kbn-xsrf', 'true')
.send({
status: 'in_progress',
guideId: 'observability',
isActive: true,
steps: [
{
id: 'add_data',
status: 'complete',
},
{
id: 'view_dashboard',
status: 'complete',
},
{
id: 'tour_observability',
status: 'in_progress',
},
],
})
.expect(200);
});
it('can complete tour', async () => {
await setInitialTourState();
it('can complete tour', async () => {
await setInitialTourState();
// Step 1: Overview
await pageObjects.infraHome.waitForTourStep('overviewStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('overviewStep');
// Step 1: Overview
await pageObjects.infraHome.waitForTourStep('overviewStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('overviewStep');
// Step 2: Streams
await pageObjects.infraHome.waitForTourStep('streamStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('streamStep');
// Step 2: Streams
await pageObjects.infraHome.waitForTourStep('streamStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('streamStep');
// Step 3: Metrics explorer
await pageObjects.infraHome.waitForTourStep('metricsExplorerStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('metricsExplorerStep');
// Step 3: Metrics explorer
await pageObjects.infraHome.waitForTourStep('metricsExplorerStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('metricsExplorerStep');
// Step 4: Services
await pageObjects.infraHome.waitForTourStep('servicesStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('servicesStep');
// Step 4: Services
await pageObjects.infraHome.waitForTourStep('servicesStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('servicesStep');
// Step 5: Alerts
await pageObjects.infraHome.waitForTourStep('alertStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('alertStep');
// Step 5: Alerts
await pageObjects.infraHome.waitForTourStep('alertStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('alertStep');
// Step 6: Guided setup
await pageObjects.infraHome.waitForTourStep('guidedSetupStep');
await pageObjects.infraHome.clickTourEndButton();
await pageObjects.infraHome.ensureTourStepIsClosed('guidedSetupStep');
});
// Step 6: Guided setup
await pageObjects.infraHome.waitForTourStep('guidedSetupStep');
await pageObjects.infraHome.clickTourEndButton();
await pageObjects.infraHome.ensureTourStepIsClosed('guidedSetupStep');
});
it('can skip tour', async () => {
await setInitialTourState();
it('can skip tour', async () => {
await setInitialTourState();
await pageObjects.infraHome.waitForTourStep('overviewStep');
await pageObjects.infraHome.clickTourSkipButton();
await pageObjects.infraHome.waitForTourStep('overviewStep');
await pageObjects.infraHome.clickTourSkipButton();
// Verify current step ("Overview") is not displayed
await pageObjects.infraHome.ensureTourStepIsClosed('overviewStep');
// Verify next step ("Streams") is not displayed
await pageObjects.infraHome.ensureTourStepIsClosed('streamStep');
// Verify current step ("Overview") is not displayed
await pageObjects.infraHome.ensureTourStepIsClosed('overviewStep');
// Verify next step ("Streams") is not displayed
await pageObjects.infraHome.ensureTourStepIsClosed('streamStep');
await browser.refresh();
await browser.refresh();
// Verify current step ("Overview") is not displayed after browser refresh,
// i.e., localStorage has been updated to not show the tour again
await pageObjects.infraHome.ensureTourStepIsClosed('overviewStep');
});
// Verify current step ("Overview") is not displayed after browser refresh,
// i.e., localStorage has been updated to not show the tour again
await pageObjects.infraHome.ensureTourStepIsClosed('overviewStep');
});
it('can start mid-tour', async () => {
await setInitialTourState(5);
it('can start mid-tour', async () => {
await setInitialTourState(5);
// Step 5: Alerts
await pageObjects.infraHome.waitForTourStep('alertStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('alertStep');
// Step 5: Alerts
await pageObjects.infraHome.waitForTourStep('alertStep');
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.ensureTourStepIsClosed('alertStep');
// Step 6: Guided setup
await pageObjects.infraHome.waitForTourStep('guidedSetupStep');
await pageObjects.infraHome.clickTourEndButton();
await pageObjects.infraHome.ensureTourStepIsClosed('guidedSetupStep');
});
// Step 6: Guided setup
await pageObjects.infraHome.waitForTourStep('guidedSetupStep');
await pageObjects.infraHome.clickTourEndButton();
await pageObjects.infraHome.ensureTourStepIsClosed('guidedSetupStep');
});
it('navigates the user to the guided setup step', async () => {
// For brevity, starting the tour at step 5
await setInitialTourState(5);
it('navigates the user to the guided setup step', async () => {
// For brevity, starting the tour at step 5
await setInitialTourState(5);
await pageObjects.infraHome.waitForTourStep('alertStep');
await pageObjects.infraHome.waitForTourStep('alertStep');
// Click on Alerts link
await (await find.byCssSelector('[data-nav-id="alerts"]')).click();
// Click on Alerts link
await (await find.byCssSelector('[data-nav-id="alerts"]')).click();
// Verify user correctly navigated to the Alerts page
const alertsPageUrl = await browser.getCurrentUrl();
expect(alertsPageUrl).to.contain('/app/observability/alerts');
// Verify user correctly navigated to the Alerts page
const alertsPageUrl = await browser.getCurrentUrl();
expect(alertsPageUrl).to.contain('/app/observability/alerts');
// Verify Step 5 persists on Alerts page, then continue with tour
await pageObjects.infraHome.waitForTourStep('alertStep');
await pageObjects.infraHome.clickTourNextButton();
// Verify Step 5 persists on Alerts page, then continue with tour
await pageObjects.infraHome.waitForTourStep('alertStep');
await pageObjects.infraHome.clickTourNextButton();
// Verify user navigated back to the overview page, and guided setup step renders (Step 6)
await pageObjects.infraHome.waitForTourStep('guidedSetupStep');
const overviewPageUrl = await browser.getCurrentUrl();
expect(overviewPageUrl).to.contain('/app/observability/overview');
});
// Verify user navigated back to the overview page, and guided setup step renders (Step 6)
await pageObjects.infraHome.waitForTourStep('guidedSetupStep');
const overviewPageUrl = await browser.getCurrentUrl();
expect(overviewPageUrl).to.contain('/app/observability/overview');
});
it('ends the tour if the user clicks on the guided setup button', async () => {
// For brevity, starting the tour at step 5, "Alerts"
await setInitialTourState(5);
it('ends the tour if the user clicks on the guided setup button', async () => {
// For brevity, starting the tour at step 5, "Alerts"
await setInitialTourState(5);
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.waitForTourStep('guidedSetupStep');
await pageObjects.infraHome.clickGuidedSetupButton();
await pageObjects.infraHome.ensureTourStepIsClosed('guidedSetupStep');
});
await pageObjects.infraHome.clickTourNextButton();
await pageObjects.infraHome.waitForTourStep('guidedSetupStep');
await pageObjects.infraHome.clickGuidedSetupButton();
await pageObjects.infraHome.ensureTourStepIsClosed('guidedSetupStep');
});
}
});
});
};