Skip to content

Commit

Permalink
address some PR feedback, including fixing the URL navigation behaviour
Browse files Browse the repository at this point in the history
  • Loading branch information
jloleysens committed Mar 16, 2021
1 parent a54861b commit 7b0da50
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@ const i18nTexts = {
};

export const ColdPhase: FunctionComponent = () => {
const {
isUsingSearchableSnapshotInHotPhase,
isUsingSearchableSnapshotInColdPhase,
} = useConfigurationIssues();
const { isUsingSearchableSnapshotInHotPhase, canUseRollupInColdPhase } = useConfigurationIssues();

return (
<Phase phase="cold" topLevelSettings={<SearchableSnapshotField phase="cold" />}>
Expand All @@ -48,9 +45,7 @@ export const ColdPhase: FunctionComponent = () => {
phase="cold"
/>

{!isUsingSearchableSnapshotInColdPhase && !isUsingSearchableSnapshotInHotPhase && (
<RollupField phase="cold" />
)}
{canUseRollupInColdPhase && <RollupField phase="cold" />}

<IndexPriorityField phase="cold" />
</Phase>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import React, { FunctionComponent } from 'react';
import { get } from 'lodash';

import { useFormData, FormHook } from '../../../../../shared_imports';

import { RollupWizard as RollupWizardView, Props as RollupWizardViewProps } from './rollup_wizard';
import { FieldChooserProvider } from './field_chooser_context';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ const deriveStepFields = (value: RollupAction['config'] | undefined): StepFields
);
};

// TODO: This component should be migrated to use the ES UI generic FormWizard component that takes care functionality for navigating to steps.
export class RollupWizard extends Component<Props, State> {
lastIndexPatternValidationTime: number;
// @ts-ignore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ export class FieldChooser extends Component<Props, State> {
return (
<Form form={customFieldForm}>
<UseField<string>
path="field"
path="fieldName"
form={customFieldForm}
component={TextField}
config={{
Expand Down Expand Up @@ -389,7 +389,7 @@ export class FieldChooser extends Component<Props, State> {
return;
}
customFieldForm.reset();
onSelectField({ name: data.field });
onSelectField({ name: data.fieldName });
}}
>
{i18n.translate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@
*/

export interface CustomFieldForm {
field: string;
fieldName: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ export const stepIds = [STEP_DATE_HISTOGRAM, STEP_TERMS, STEP_HISTOGRAM, STEP_ME
* Map a specific wizard step to two functions:
* 1. getDefaultFields: (overrides) => object
* 2. fieldValidations
*
* See rollup/public/crud_app/services/jobs.js for more information on override's shape
*/
export const stepIdToStepConfigMap = {
[STEP_DATE_HISTOGRAM]: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export const EditPolicy: React.FunctionComponent<Props & RouteComponentProps<Rou
}

const maybePolicy = getPolicyByName(policies, attemptToURIDecode(policyName))?.policy;
const isNewPolicy = !maybePolicy;
const isNewPolicy = maybePolicy === undefined;
const policy = maybePolicy ?? defaultPolicy;

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,16 +145,6 @@ export const EditPolicy: React.FunctionComponent<Props> = ({ history }) => {
window.scrollTo(0, 0);
}, []);

const currentViewPhase = currentView.id === 'rollupAction' ? currentView.phase : undefined;
const currentRollupField = form.getFields()[`phases.${currentViewPhase}.actions.rollup.config`];
const hasCurrentRollupField = Boolean(currentRollupField);

useEffect(() => {
if (!hasCurrentRollupField) {
goToPolicyView();
}
}, [hasCurrentRollupField, goToPolicyView]);

return (
<EuiPage>
<EuiPageBody>
Expand Down Expand Up @@ -351,7 +341,7 @@ export const EditPolicy: React.FunctionComponent<Props> = ({ history }) => {
) : null}
</Form>
</div>
{currentView.id === 'rollupAction' && hasCurrentRollupField && (
{currentView.id === 'rollupAction' && (
<RollupWizard
form={form}
policyName={currentPolicyName || originalPolicyName}
Expand All @@ -362,14 +352,14 @@ export const EditPolicy: React.FunctionComponent<Props> = ({ history }) => {
}}
onDone={(rollupAction) => {
const { phase } = currentView;
const fieldPath = `phases.${phase}.actions.rollup.config`;
const rollupField = form.getFields()[fieldPath];
const rollupConfigPath = `phases.${phase}.actions.rollup.config`;
const rollupField = form.getFields()[rollupConfigPath];
const newCurrentPolicy = cloneDeep({
...currentPolicy,
name: originalPolicyName,
});

set(newCurrentPolicy, fieldPath, rollupAction);
set(newCurrentPolicy, rollupConfigPath, rollupAction);
setCurrentPolicy(newCurrentPolicy);
rollupField.setValue(rollupAction);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
*/

import { get } from 'lodash';
import React, { FunctionComponent, createContext, useContext } from 'react';
import React, { FunctionComponent, createContext, useContext, useEffect } from 'react';

import { useFormData } from '../../../../shared_imports';

import { isUsingDefaultRolloverPath, isUsingCustomRolloverPath } from '../constants';

import { useNavigationContext } from '../navigation';

export interface ConfigurationIssues {
/**
* Whether the serialized policy will use rollover. This blocks certain actions in
Expand All @@ -26,6 +28,8 @@ export interface ConfigurationIssues {
*/
isUsingSearchableSnapshotInHotPhase: boolean;
isUsingSearchableSnapshotInColdPhase: boolean;

canUseRollupInColdPhase: boolean;
}

const ConfigurationIssuesContext = createContext<ConfigurationIssues>(null as any);
Expand All @@ -37,6 +41,7 @@ const pathToColdPhaseSearchableSnapshot =
'phases.cold.actions.searchable_snapshot.snapshot_repository';

export const ConfigurationIssuesProvider: FunctionComponent = ({ children }) => {
const { setIsColdRollupActionBlocked } = useNavigationContext();
const [formData] = useFormData({
watch: [
pathToHotPhaseSearchableSnapshot,
Expand All @@ -49,14 +54,25 @@ export const ConfigurationIssuesProvider: FunctionComponent = ({ children }) =>
// Provide default value, as path may become undefined if removed from the DOM
const isUsingCustomRollover = get(formData, isUsingCustomRolloverPath, true);

const isUsingSearchableSnapshotInHotPhase =
get(formData, pathToHotPhaseSearchableSnapshot) != null;
const isUsingSearchableSnapshotInColdPhase =
get(formData, pathToColdPhaseSearchableSnapshot) != null;

const canUseRollupInColdPhase =
!isUsingSearchableSnapshotInHotPhase && !isUsingSearchableSnapshotInColdPhase;

useEffect(() => {
setIsColdRollupActionBlocked(!canUseRollupInColdPhase);
}, [setIsColdRollupActionBlocked, canUseRollupInColdPhase]);

return (
<ConfigurationIssuesContext.Provider
value={{
isUsingRollover: isUsingDefaultRollover === false ? isUsingCustomRollover : true,
isUsingSearchableSnapshotInHotPhase:
get(formData, pathToHotPhaseSearchableSnapshot) != null,
isUsingSearchableSnapshotInColdPhase:
get(formData, pathToColdPhaseSearchableSnapshot) != null,
isUsingSearchableSnapshotInHotPhase,
isUsingSearchableSnapshotInColdPhase,
canUseRollupInColdPhase,
}}
>
{children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import React, {
FunctionComponent,
createContext,
useState,
useContext,
useMemo,
useEffect,
Expand All @@ -27,16 +28,16 @@ export type PolicyView = { id: 'policy' } | { id: 'rollupAction'; phase: 'hot' |
interface ContextValue {
currentView: PolicyView;
goToPolicyView: (scrollToFieldWithId?: string) => void;
setIsColdRollupActionBlocked: (value: boolean) => void;
}

const NavigationContext = createContext<ContextValue>({
currentView: { id: 'policy' },
goToPolicyView: () => {},
});
const NavigationContext = createContext<ContextValue | null>(null);

export const NavigationContextProvider: FunctionComponent = ({ children }) => {
const { policyName } = useEditPolicyContext();

const [isColdRollupActionBlocked, setIsColdRollupActionBlocked] = useState(true);

const { search } = useLocation();
const history = useHistory();

Expand All @@ -45,12 +46,14 @@ export const NavigationContextProvider: FunctionComponent = ({ children }) => {
} = useKibana();

const currentView = useMemo<PolicyView>(() => {
const { rollup } = qs.parse(search) as { rollup: 'hot' | 'cold' };
if (rollup) {
const { rollup } = qs.parse(search) as { rollup?: 'hot' | 'cold' };
const rollupBlocked = rollup === 'cold' && isColdRollupActionBlocked;
if (rollup && !rollupBlocked) {
return { id: 'rollupAction', phase: rollup };
}

return { id: 'policy' };
}, [search]);
}, [search, isColdRollupActionBlocked]);

const { id: currentViewId } = currentView;

Expand Down Expand Up @@ -83,7 +86,9 @@ export const NavigationContextProvider: FunctionComponent = ({ children }) => {
}, [breadcrumbService, search, currentViewId, policyName]);

return (
<NavigationContext.Provider value={{ currentView, goToPolicyView }}>
<NavigationContext.Provider
value={{ currentView, goToPolicyView, setIsColdRollupActionBlocked }}
>
{children}
</NavigationContext.Provider>
);
Expand Down

0 comments on commit 7b0da50

Please sign in to comment.