Skip to content

Commit

Permalink
Merge pull request #5988 from TheRealJon/1856867-bz
Browse files Browse the repository at this point in the history
[release-4.5] Bug 1856867: Do not prune empty values from sample on create operand page
  • Loading branch information
openshift-merge-robot committed Aug 8, 2020
2 parents fc61f29 + da969f8 commit 49d19ad
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,45 @@ export const getSchemaErrors = (schema: JSONSchema6): SchemaError[] => {
];
};

// Returns true if a value is not nil and is empty
const definedAndEmpty = (value) => !_.isNil(value) && _.isEmpty(value);

// Helper function for prune
// TODO (jon) Make this pure
const pruneRecursive = (current: any, sample: any): any => {
const valueIsEmpty = (value, key) =>
_.isNil(value) ||
_.isNaN(value) ||
(_.isString(value) && _.isEmpty(value)) ||
(_.isObject(value) && _.isEmpty(pruneRecursive(value, sample?.[key])));

// Value should be pruned if it is empty and the correspondeing sample is not explicitly
// defined as an empty value.
const shouldPrune = (value, key) => valueIsEmpty(value, key) && !definedAndEmpty(sample?.[key]);

// Prune each property of current value that meets the pruning criteria
_.forOwn(current, (value, key) => {
if (shouldPrune(value, key)) {
delete current[key];
}
});

// remove any leftover undefined values from the delete operation on an array
if (_.isArray(current)) {
_.pull(current, undefined);
}

return current;
};

// Deeply remove all empty, NaN, null, or undefined values from an object or array. If a value meets
// the above criteria, but the corresponding sample is explicitly defined as an empty vaolue, it
// will not be pruned.
// Based on https://stackoverflow.com/a/26202058/8895304
export const prune = (obj: any, sample?: any): any => {
return pruneRecursive(_.cloneDeep(obj), sample);
};

type SchemaError = {
title: string;
message: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as React from 'react';
import * as _ from 'lodash';
import { K8sResourceKind } from '@console/internal/module/k8s';
import { EditorType, EditorToggle } from './editor-toggle';
import { prune } from '../../utils';
import { safeJSToYAML, asyncYAMLToJS } from '../../utils/yaml';
import { Alert, Button } from '@patternfly/react-core';

Expand Down Expand Up @@ -32,6 +31,7 @@ export const SyncedEditor: React.FC<SyncedEditorProps> = ({
initialData = {},
onChangeEditorType = _.noop,
onChange = _.noop,
prune,
YAMLEditor,
}) => {
const { formContext, yamlContext } = context;
Expand Down Expand Up @@ -71,7 +71,7 @@ export const SyncedEditor: React.FC<SyncedEditorProps> = ({
};

const handleToggleToYAML = () => {
setYAML(safeJSToYAML({ ...formData, spec: prune(formData.spec) }, yaml, YAML_TO_JS_OPTIONS));
setYAML(safeJSToYAML(prune?.(formData) ?? formData, yaml, YAML_TO_JS_OPTIONS));
changeEditorType(EditorType.YAML);
};

Expand Down Expand Up @@ -119,7 +119,12 @@ export const SyncedEditor: React.FC<SyncedEditorProps> = ({
</Alert>
)}
{type === EditorType.Form ? (
<FormEditor formData={formData} onChange={handleFormDataChange} {...formContext} />
<FormEditor
formData={formData}
onChange={handleFormDataChange}
prune={prune}
{...formContext}
/>
) : (
<YAMLEditor initialYAML={yaml} onChange={handleYAMLChange} {...yamlContext} />
)}
Expand All @@ -137,5 +142,6 @@ type SyncedEditorProps = {
initialData?: K8sResourceKind;
onChangeEditorType?: (newType: EditorType) => void;
onChange?: (data: K8sResourceKind) => void;
prune?: (data: any) => any;
YAMLEditor: React.FC<any>;
};
30 changes: 0 additions & 30 deletions frontend/packages/console-shared/src/utils/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import * as _ from 'lodash';
import { K8sResourceKind } from '@console/internal/module/k8s';
import { FirehoseResult } from '@console/internal/components/utils/types';
import { getUID } from '../selectors/common';
Expand Down Expand Up @@ -34,32 +33,3 @@ export const getRandomChars = (len = 6): string => {
.replace(/[^a-z0-9]+/g, '')
.substr(1, len);
};

// Recursively remove all empty, NaN, null, or undefined values from an object or array.
// Based on https://stackoverflow.com/a/26202058/8895304
const pruneRecursive = (current) => {
_.forOwn(current, (value, key) => {
if (
_.isNil(value) ||
_.isNaN(value) ||
(_.isString(value) && _.isEmpty(value)) ||
(_.isObject(value) && _.isEmpty(pruneRecursive(value)))
) {
delete current[key];
}
});

// remove any leftover undefined values from the delete
// operation on an array
if (_.isArray(current)) {
_.pull(current, undefined);
}

return current;
};

// Deeply remove all empty, NaN, null, or undefined values from an object or array.
// Based on https://stackoverflow.com/a/26202058/8895304
export const prune = (obj) => {
return pruneRecursive(_.cloneDeep(obj));
};
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import * as _ from 'lodash';
import * as React from 'react';
import { JSONSchema6 } from 'json-schema';
import {
K8sKind,
Expand All @@ -21,8 +23,6 @@ import { RootState } from '@console/internal/redux';
import { SyncedEditor } from '@console/shared/src/components/synced-editor';
import { getActivePerspective } from '@console/internal/reducers/ui';
import { EditorType } from '@console/shared/src/components/synced-editor/editor-toggle';
import * as _ from 'lodash';
import * as React from 'react';
import { connect } from 'react-redux';
import { Helmet } from 'react-helmet';
import { match as RouterMatch } from 'react-router';
Expand All @@ -32,7 +32,7 @@ import { OperandForm } from './operand-form';
import { OperandYAML } from './operand-yaml';
import { exampleForModel, providedAPIForModel } from '..';
import { FORM_HELP_TEXT, YAML_HELP_TEXT, DEFAULT_K8S_SCHEMA } from './const';
import { getSchemaErrors } from '@console/shared/src/components/dynamic-form/utils';
import { getSchemaErrors, prune } from '@console/shared/src/components/dynamic-form/utils';
import { hasNoFields } from './utils';
// eslint-disable-next-line @typescript-eslint/camelcase
import { DEPRECATED_CreateOperandForm } from './DEPRECATED_operand-form';
Expand Down Expand Up @@ -87,6 +87,8 @@ export const CreateOperand: React.FC<CreateOperandProps> = ({

const sample = React.useMemo<K8sResourceKind>(() => exampleForModel(csv, model), [csv, model]);

const pruneFunc = React.useCallback((data) => prune(data, sample), [sample]);

const onChangeEditorType = React.useCallback((newMethod) => {
setHelpText(newMethod === EditorType.Form ? FORM_HELP_TEXT : YAML_HELP_TEXT);
}, []);
Expand Down Expand Up @@ -123,6 +125,7 @@ export const CreateOperand: React.FC<CreateOperandProps> = ({
initialData={sample}
initialType={initialEditorType}
onChangeEditorType={onChangeEditorType}
prune={pruneFunc}
YAMLEditor={OperandYAML}
/>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { ClusterServiceVersionKind, CRDDescription, APIServiceDefinition } from
import { ClusterServiceVersionLogo } from '../index';
import { DynamicForm } from '@console/shared/src/components/dynamic-form';
import { getUISchema } from './utils';
import { prune } from '@console/shared';

export const OperandForm: React.FC<OperandFormProps> = ({
csv,
Expand All @@ -17,20 +16,21 @@ export const OperandForm: React.FC<OperandFormProps> = ({
next,
onChange,
providedAPI,
prune,
schema,
}) => {
const [errors, setErrors] = React.useState<string[]>([]);
// const [formData, setFormData] = React.useState(initialData);

const processFormData = ({ metadata, spec, ...rest }) => {
return {
const processFormData = ({ metadata, ...rest }) => {
const data = {
metadata: {
...metadata,
...(match?.params?.ns && model.namespaced && { namespace: match.params.ns }),
},
spec: prune(spec),
...rest,
};
return prune?.(data) ?? data;
};

const handleSubmit = ({ formData: submitFormData }) => {
Expand Down Expand Up @@ -86,5 +86,6 @@ export type OperandFormProps = {
csv: ClusterServiceVersionKind;
model: K8sKind;
providedAPI: ProvidedAPI;
prune?: (data: any) => any;
schema: JSONSchema6;
};

0 comments on commit 49d19ad

Please sign in to comment.