Skip to content

Commit

Permalink
Fix after review
Browse files Browse the repository at this point in the history
  • Loading branch information
nchaulet committed Jun 28, 2022
1 parent 4f6e1e0 commit 3bff578
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export function FormWizard<T extends object = { [key: string]: any }, S extends
isSaving,
onSave,
onChange,
onStepChange,
children,
rightContentNav,
}: Props<T, S>) {
Expand All @@ -41,6 +42,7 @@ export function FormWizard<T extends object = { [key: string]: any }, S extends
isEditing={isEditing}
onSave={onSave}
onChange={onChange}
onStepChange={onStepChange}
defaultActiveStep={defaultActiveStep}
>
<FormWizardConsumer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface Props<T extends object> {
defaultActiveStep?: number;
defaultValue?: HookProps<T>['defaultValue'];
onChange?: HookProps<T>['onChange'];
onStepChange?: (id: string) => void;
}

interface State {
Expand Down Expand Up @@ -48,7 +49,7 @@ const formWizardContext = createContext<Context>({} as Context);

export const FormWizardProvider = WithMultiContent<Props<any>>(function FormWizardProvider<
T extends object = { [key: string]: any }
>({ children, defaultActiveStep = 0, isEditing, onSave }: Props<T>) {
>({ children, defaultActiveStep = 0, isEditing, onSave, onStepChange }: Props<T>) {
const { getData, validate, validation } = useMultiContentContext<T>();

const [state, setState] = useState<State>({
Expand Down Expand Up @@ -135,8 +136,12 @@ export const FormWizardProvider = WithMultiContent<Props<any>>(function FormWiza

return nextState;
});
// Trigger onStepChange
if (onStepChange) {
onStepChange(Object.values(state.steps)[getStepIndex(stepId)]?.id);
}
},
[getStepIndex, validate, onSave, getData, lastStep]
[getStepIndex, validate, onSave, onStepChange, getData, lastStep, state.steps]
);

const value: Context = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export interface AppDependencies {
fatalErrors: FatalErrorsStart;
getUrlForApp: ApplicationStart['getUrlForApp'];
executionContext: ExecutionContextStart;
application: ApplicationStart;
};
plugins: {
usageCollection: UsageCollectionSetup;
Expand All @@ -41,7 +42,6 @@ export interface AppDependencies {
extensionsService: ExtensionsService;
httpService: HttpService;
notificationService: NotificationService;
application: ApplicationStart;
};
history: ScopedHistory;
setBreadcrumbs: ManagementAppMountParams['setBreadcrumbs'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ describe('<ComponentTemplateEdit />', () => {

it('should allow to go directly to a step', async () => {
await act(async () => {
testBed = await setup(httpSetup, '?tab=mappings');
testBed = await setup(httpSetup, '?step=mappings');
});

testBed.component.update();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,61 @@

import { renderHook } from '@testing-library/react-hooks';
import { createMemoryHistory } from 'history';
import { useTabFromQueryString } from './component_template_edit';
import { useStepFromQueryString } from './component_template_edit';

describe('useTabFromQueryString', () => {
it('should return undefined if not tab is set in the url', () => {
it('should return undefined if no step is set in the url', () => {
const history = createMemoryHistory();
history.push('/app/management/data/index_management/edit_component_template');

const {
result: { current: tab },
} = renderHook(() => useTabFromQueryString(history));
result: {
current: { activeStep },
},
} = renderHook(() => useStepFromQueryString(history));

expect(tab).not.toBeDefined();
expect(activeStep).not.toBeDefined();
});

it('should return the tab if set in the url', () => {
it('should return the step if set in the url', () => {
const history = createMemoryHistory();
history.push('/app/management/data/index_management/edit_component_template?tab=mappings');
history.push('/app/management/data/index_management/edit_component_template?step=mappings');

const {
result: { current: tab },
} = renderHook(() => useTabFromQueryString(history));
result: {
current: { activeStep },
},
} = renderHook(() => useStepFromQueryString(history));

expect(tab).toBe('mappings');
expect(activeStep).toBe('mappings');
});

it('should not update history on step change if no step is set in the url', () => {
const history = createMemoryHistory();
history.push('/app/management/data/index_management/edit_component_template');

const {
result: {
current: { updateStep },
},
} = renderHook(() => useStepFromQueryString(history));

updateStep('aliases');

expect(history.location.search).toBe('');
});

it('should update history on step change if a step is set in the url', () => {
const history = createMemoryHistory();
history.push('/app/management/data/index_management/edit_component_template?step=mappings');

const {
result: {
current: { updateStep },
},
} = renderHook(() => useStepFromQueryString(history));

updateStep('aliases');
expect(history.location.search).toBe('?step=aliases');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React, { useState, useEffect, useMemo } from 'react';
import React, { useState, useEffect, useMemo, useCallback } from 'react';
import { RouteComponentProps } from 'react-router-dom';
import { FormattedMessage } from '@kbn/i18n-react';
import { EuiPageContentBody, EuiPageHeader, EuiSpacer } from '@elastic/eui';
Expand All @@ -27,13 +27,28 @@ interface MatchParams {
name: string;
}

export function useTabFromQueryString(history: History): WizardSection | undefined {
return useMemo(() => {
export function useStepFromQueryString(history: History) {
const activeStep = useMemo(() => {
const params = new URLSearchParams(history.location.search);
if (params.has('tab')) {
return params.get('tab') as WizardSection;
if (params.has('step')) {
return params.get('step') as WizardSection;
}
}, [history.location.search]);

const updateStep = useCallback(
(stepId: string) => {
const params = new URLSearchParams(history.location.search);
if (params.has('step')) {
params.set('step', stepId);
history.push({
search: params.toString(),
});
}
},
[history]
);

return { activeStep, updateStep };
}

export const ComponentTemplateEdit: React.FunctionComponent<RouteComponentProps<MatchParams>> = ({
Expand All @@ -43,7 +58,7 @@ export const ComponentTemplateEdit: React.FunctionComponent<RouteComponentProps<
history,
}) => {
const { api, breadcrumbs } = useComponentTemplatesContext();
const defaultActiveStep = useTabFromQueryString(history);
const { activeStep: defaultActiveStep, updateStep } = useStepFromQueryString(history);
const redirectTo = useRedirectPath(history);

const [isSaving, setIsSaving] = useState<boolean>(false);
Expand Down Expand Up @@ -126,7 +141,8 @@ export const ComponentTemplateEdit: React.FunctionComponent<RouteComponentProps<

<ComponentTemplateForm
defaultValue={componentTemplate!}
defaultActiveStep={defaultActiveStep}
defaultActiveWizardSection={defaultActiveStep}
onStepChange={updateStep}
onSave={onSave}
isSaving={isSaving}
saveError={saveError}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ interface Props {
saveError: any;
defaultValue?: ComponentTemplateDeserialized;
isEditing?: boolean;
defaultActiveStep?: WizardSection;
defaultActiveWizardSection?: WizardSection;
onStepChange?: (stepId: string) => void;
}

const wizardSections: { [id: string]: { id: WizardSection; label: string } } = {
Expand Down Expand Up @@ -88,8 +89,9 @@ export const ComponentTemplateForm = ({
isSaving,
saveError,
clearSaveError,
defaultActiveStep,
defaultActiveWizardSection,
onSave,
onStepChange,
}: Props) => {
const {
template: { settings, mappings, aliases },
Expand Down Expand Up @@ -198,8 +200,13 @@ export const ComponentTemplateForm = ({

const defaultActiveStepIndex = useMemo(
() =>
Math.max(defaultActiveStep ? Object.keys(wizardSections).indexOf(defaultActiveStep) : 0, 0),
[defaultActiveStep]
Math.max(
defaultActiveWizardSection
? Object.keys(wizardSections).indexOf(defaultActiveWizardSection)
: 0,
0
),
[defaultActiveWizardSection]
);

return (
Expand All @@ -211,6 +218,7 @@ export const ComponentTemplateForm = ({
apiError={apiError}
texts={i18nTexts}
defaultActiveStep={defaultActiveStepIndex}
onStepChange={onStepChange}
>
<FormWizardStep
id={wizardSections.logistics.id}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { renderHook } from '@testing-library/react-hooks';
import { coreMock } from '@kbn/core/public/mocks';
import { createMemoryHistory } from 'history';
import { useRedirectPath } from './redirect_path';
import { useKibana } from '..';
Expand All @@ -14,14 +15,12 @@ const mockedUseKibana = useKibana as jest.MockedFunction<typeof useKibana>;
jest.mock('..');

describe('useRedirectPath', () => {
const mockedNavigateToUrl = jest.fn();
const mockStart = coreMock.createStart();
beforeEach(() => {
mockedNavigateToUrl.mockReset();
mockStart.application.navigateToUrl.mockReset();
mockedUseKibana.mockReturnValue({
services: {
application: {
navigateToUrl: mockedNavigateToUrl,
},
core: {
application: mockStart.application,
},
} as any);
});
Expand All @@ -37,7 +36,7 @@ describe('useRedirectPath', () => {

redirectToPathOrRedirectPath({ pathname: '/test' });

expect(mockedNavigateToUrl).toBeCalledWith('/test-redirect-path');
expect(mockStart.application.navigateToUrl).toBeCalledWith('/test-redirect-path');
});

it('should redirect to the provided path if no redirect path is specified in the url', () => {
Expand All @@ -50,7 +49,7 @@ describe('useRedirectPath', () => {

redirectToPathOrRedirectPath({ pathname: '/test' });

expect(mockedNavigateToUrl).not.toBeCalled();
expect(mockStart.application.navigateToUrl).not.toBeCalled();
expect(history.location.pathname).toBe('/test');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export async function mountManagementSection(
fatalErrors,
getUrlForApp: application.getUrlForApp,
executionContext,
application,
},
plugins: {
usageCollection,
Expand All @@ -91,7 +92,6 @@ export async function mountManagementSection(
notificationService,
uiMetricService,
extensionsService,
application,
},
history,
setBreadcrumbs,
Expand Down

0 comments on commit 3bff578

Please sign in to comment.