github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 17:59:23 -04:00
Code Issues Projects Releases Packages Wiki Activity

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

Browse source 
This reverts commit 02a2a6a500.
This commit is contained in:
Tyler Smalley 2022-11-02 09:44:16 -07:00
parent 20d91f1be0
commit a4445f959d
7 changed files with 149 additions and 260 deletions
Download patch file Download diff file Expand all files Collapse all files

9
packages/kbn-guided-onboarding/index.ts
View file

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

17
src/plugins/guided_onboarding/public/components/guide_panel.test.tsx
View file

@ -109,21 +109,10 @@ describe('Guided setup', () => {
});
describe('Button component', () => {
test('should be hidden in there is no guide state', async () => {
// 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 () => {
const { exists } = testBed;
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('disabledGuideButton')).toBe(true);
expect(exists('guideButton')).toBe(false);
expect(exists('guidePanel')).toBe(false);
});

2
src/plugins/guided_onboarding/public/components/guide_panel.tsx
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 || !guideState || !guideState.isActive) {
if (!guideConfig) {
// TODO button show/hide logic https://github.com/elastic/kibana/issues/141129
return null;
}

18
src/plugins/guided_onboarding/public/services/api.mocks.ts
View file

@ -77,24 +77,6 @@ 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,

95
src/plugins/guided_onboarding/public/services/api.test.ts
View file

@ -11,7 +11,6 @@ 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 {
@ -25,14 +24,12 @@ 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' });
@ -44,8 +41,9 @@ describe('GuidedOnboarding ApiService', () => {
});
afterEach(() => {
subscription?.unsubscribe();
anotherSubscription?.unsubscribe();
if (subscription) {
subscription.unsubscribe();
}
jest.restoreAllMocks();
});
@ -55,64 +53,6 @@ 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();
}
});
});
@ -155,17 +95,6 @@ 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$', () => {
@ -220,6 +149,24 @@ 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);
});

57
src/plugins/guided_onboarding/public/services/api.ts
View file

@ -7,7 +7,7 @@
*/
import { HttpSetup } from '@kbn/core/public';
import { BehaviorSubject, map, concatMap, of, Observable, firstValueFrom } from 'rxjs';
import { BehaviorSubject, map, from, concatMap, of, Observable, firstValueFrom } from 'rxjs';
import type { GuideState, GuideId, GuideStep, GuideStepIds } from '@kbn/guided-onboarding';
import { GuidedOnboardingApi } from '../types';
@ -26,7 +26,6 @@ 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 {
@ -34,46 +33,30 @@ 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 && !this.isGuideStateLoading ? this.createGetStateObservable() : of(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)
)
);
}
@ -100,7 +83,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} newState the updated guide state
* @param {GuideState} guideState 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
*/
@ -116,8 +99,8 @@ export class ApiService implements GuidedOnboardingApi {
const response = await this.client.put<{ state: GuideState }>(`${API_BASE_PATH}/state`, {
body: JSON.stringify(newState),
});
// broadcast the newState
this.onboardingGuideState$.next(newState);
// If the guide has been deactivated, we return undefined
this.onboardingGuideState$.next(newState.isActive ? newState : undefined);
this.isGuidePanelOpen$.next(panelState);
return response;
} catch (error) {

211
x-pack/test/functional/apps/infra/tour.ts
View file

@ -15,7 +15,6 @@ 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));
@ -24,9 +23,8 @@ 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
@ -39,134 +37,131 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => {
});
describe('Tour enabled', () => {
// 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);
});
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');
});
});
});
};