Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFR] Fix Forms Remove Empty Values #3758

Merged
merged 1 commit into from
Oct 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions cypress/integration/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,18 @@ describe('Edit Page', () => {
expect(el).to.have.value(date)
);
});

it('should persit emptied inputs', () => {
EditPostPage.navigate();
EditPostPage.gotoTab(3);
cy.contains('Tech').click();
cy.get('li[aria-label="Clear value"]').click();
EditPostPage.submit();

EditPostPage.navigate();
EditPostPage.gotoTab(3);
cy.get(EditPostPage.elements.input('category')).should(el =>
expect(el).to.have.value('')
);
});
});
1 change: 1 addition & 0 deletions examples/simple/src/posts/PostEdit.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ const PostEdit = ({ permissions, ...props }) => (
</ArrayInput>
<DateInput source="published_at" options={{ locale: 'pt' }} />
<SelectInput
allowEmpty
resettable
source="category"
choices={[
Expand Down
2 changes: 2 additions & 0 deletions packages/ra-core/src/form/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import FormField from './FormField';
import useInput, { InputProps } from './useInput';
import ValidationError from './ValidationError';
import useInitializeFormWithRecord from './useInitializeFormWithRecord';
import sanitizeEmptyValues from './sanitizeEmptyValues';
import useChoices, {
ChoicesProps,
OptionTextElement,
Expand All @@ -19,6 +20,7 @@ export {
InputProps,
OptionTextElement,
OptionText,
sanitizeEmptyValues,
useChoices,
useInput,
useInitializeFormWithRecord,
Expand Down
33 changes: 33 additions & 0 deletions packages/ra-core/src/form/sanitizeEmptyValues.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import merge from 'lodash/merge';

/**
* Because final-form removes undefined and empty string values completely
* (the key for the empty field is removed from the values), we have to check
* wether this value was initially provided so that it is correctly sent to
* the backend.
* See https://github.com/final-form/react-final-form/issues/130#issuecomment-493447888
*
* @param initialValues The initial values provided to the form
* @param values The current form values
*/
const sanitizeEmptyValues = (initialValues: object, values: object) => {
// For every field initialy provided, we check wether it value has been removed
// and set it explicitly to an empty string
const initialValuesWithEmptyFields = Object.keys(initialValues).reduce(
(acc, key) => {
if (typeof values[key] === 'object' && values[key] !== null) {
acc[key] = sanitizeEmptyValues(initialValues[key], values[key]);
} else {
acc[key] =
typeof values[key] === 'undefined' ? '' : values[key];
}
Comment on lines +18 to +23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand you consider 3 cases:

  • values[key] is undefined, in that case you initialize the value with ""
  • values[key] is a non-null object, and you recursively sanitarize the values
  • in all other cases, the value is kept as it is
    I think you should also test that initialValues[key]!==null otherwise a recursive call may occurs and throw an error (Object.keys(null) is not defined)
    Furthermore, if value[key]===nulll you keep the null value, is that the wanted behavior ?

return acc;
},
{}
);

// Finaly, we merge back the values to not miss any which wasn't initialy provided
return merge(initialValuesWithEmptyFields, values);
};

export default sanitizeEmptyValues;
23 changes: 16 additions & 7 deletions packages/ra-ui-materialui/src/form/SimpleForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import PropTypes from 'prop-types';
import { Form } from 'react-final-form';
import arrayMutators from 'final-form-arrays';
import classnames from 'classnames';
import { useTranslate, useInitializeFormWithRecord } from 'ra-core';
import {
useTranslate,
useInitializeFormWithRecord,
sanitizeEmptyValues,
} from 'ra-core';

import getFormInitialValues from './getFormInitialValues';
import FormInput from './FormInput';
Expand All @@ -20,20 +24,25 @@ const SimpleForm = ({ initialValues, defaultValue, saving, ...props }) => {
};

const translate = useTranslate();

const finalInitialValues = getFormInitialValues(
initialValues,
defaultValue,
props.record
);

const submit = values => {
const finalRedirect =
typeof redirect === undefined ? props.redirect : redirect.current;
props.save(values, finalRedirect);
const finalValues = sanitizeEmptyValues(finalInitialValues, values);

props.save(finalValues, finalRedirect);
};

return (
<Form
key={props.version}
initialValues={getFormInitialValues(
initialValues,
defaultValue,
props.record
)}
initialValues={finalInitialValues}
onSubmit={submit}
mutators={{ ...arrayMutators }}
keepDirtyOnReinitialize
Expand Down
22 changes: 15 additions & 7 deletions packages/ra-ui-materialui/src/form/TabbedForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import arrayMutators from 'final-form-arrays';
import { Route } from 'react-router-dom';
import Divider from '@material-ui/core/Divider';
import { makeStyles } from '@material-ui/core/styles';
import { useTranslate, useInitializeFormWithRecord } from 'ra-core';
import {
useTranslate,
useInitializeFormWithRecord,
sanitizeEmptyValues,
} from 'ra-core';

import getFormInitialValues from './getFormInitialValues';
import Toolbar from './Toolbar';
Expand Down Expand Up @@ -34,20 +38,24 @@ const TabbedForm = ({ initialValues, defaultValue, saving, ...props }) => {
const translate = useTranslate();
const classes = useStyles();

const finalInitialValues = getFormInitialValues(
initialValues,
defaultValue,
props.record
);

const submit = values => {
const finalRedirect =
typeof redirect === undefined ? props.redirect : redirect.current;
props.save(values, finalRedirect);
const finalValues = sanitizeEmptyValues(finalInitialValues, values);

props.save(finalValues, finalRedirect);
};

return (
<Form
key={props.version}
initialValues={getFormInitialValues(
initialValues,
defaultValue,
props.record
)}
initialValues={finalInitialValues}
onSubmit={submit}
mutators={{ ...arrayMutators }}
setRedirect={setRedirect}
Expand Down
7 changes: 6 additions & 1 deletion packages/ra-ui-materialui/src/input/SelectInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,12 @@ const SelectInput: FunctionComponent<
{...sanitizeRestProps(rest)}
>
{allowEmpty ? (
<MenuItem value={emptyValue} key="null">
<MenuItem
value={emptyValue}
key="null"
aria-label={translate('ra.action.clear_input_value')}
title={translate('ra.action.clear_input_value')}
>
{renderEmptyItemOption()}
</MenuItem>
) : null}
Expand Down