-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
fix: omitExtraData on submit and on validateForm #4228
Changes from 6 commits
c64e122
917e6ac
a262f7c
bdc4f06
a46cd70
602780a
8fe05b6
52d277a
4605f28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3291,7 +3291,7 @@ describe('Form omitExtraData and liveOmit', () => { | |
sandbox.restore(); | ||
}); | ||
|
||
it('should call getUsedFormData when the omitExtraData prop is true and liveOmit is true', () => { | ||
it('should call omitExtraData when the omitExtraData prop is true and liveOmit is true', () => { | ||
const schema = { | ||
type: 'object', | ||
properties: { | ||
|
@@ -3316,18 +3316,18 @@ describe('Form omitExtraData and liveOmit', () => { | |
liveOmit, | ||
}); | ||
|
||
sandbox.stub(comp.ref.current, 'getUsedFormData').returns({ | ||
sandbox.stub(comp.ref.current, 'omitExtraData').returns({ | ||
foo: '', | ||
}); | ||
|
||
fireEvent.change(node.querySelector('[type=text]'), { | ||
target: { value: 'new' }, | ||
}); | ||
|
||
sinon.assert.calledOnce(comp.ref.current.getUsedFormData); | ||
sinon.assert.calledOnce(comp.ref.current.omitExtraData); | ||
}); | ||
|
||
it('should not call getUsedFormData when the omitExtraData prop is true and liveOmit is unspecified', () => { | ||
it('should not call omitExtraData when the omitExtraData prop is true and liveOmit is unspecified', () => { | ||
const schema = { | ||
type: 'object', | ||
properties: { | ||
|
@@ -3349,19 +3349,19 @@ describe('Form omitExtraData and liveOmit', () => { | |
omitExtraData, | ||
}); | ||
|
||
sandbox.stub(comp.ref.current, 'getUsedFormData').returns({ | ||
sandbox.stub(comp.ref.current, 'omitExtraData').returns({ | ||
foo: '', | ||
}); | ||
|
||
fireEvent.change(node.querySelector('[type=text]'), { | ||
target: { value: 'new' }, | ||
}); | ||
|
||
sinon.assert.notCalled(comp.ref.current.getUsedFormData); | ||
sinon.assert.notCalled(comp.ref.current.omitExtraData); | ||
}); | ||
|
||
describe('getUsedFormData', () => { | ||
it('should call getUsedFormData when the omitExtraData prop is true', () => { | ||
describe('omitExtraData on submit', () => { | ||
it('should call omitExtraData when the omitExtraData prop is true', () => { | ||
Comment on lines
+3363
to
+3364
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It may be useful to keep testing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this is just verifying that it's called when the prop is set. Method is unit tested below |
||
const schema = { | ||
type: 'object', | ||
properties: { | ||
|
@@ -3385,14 +3385,65 @@ describe('Form omitExtraData and liveOmit', () => { | |
omitExtraData, | ||
}); | ||
|
||
sandbox.stub(comp.ref.current, 'getUsedFormData').returns({ | ||
sandbox.stub(comp.ref.current, 'omitExtraData').returns({ | ||
foo: '', | ||
}); | ||
|
||
fireEvent.submit(node); | ||
|
||
sinon.assert.calledOnce(comp.ref.current.getUsedFormData); | ||
sinon.assert.calledOnce(comp.ref.current.omitExtraData); | ||
}); | ||
|
||
it('Should call validateFormWithFormData with the current formData if omitExtraData is false', () => { | ||
const omitExtraData = false; | ||
const schema = { | ||
type: 'object', | ||
properties: { | ||
foo: { type: 'string' }, | ||
}, | ||
}; | ||
const formData = { foo: 'bar', baz: 'baz' }; | ||
const formRef = React.createRef(); | ||
const props = { | ||
ref: formRef, | ||
schema, | ||
formData, | ||
omitExtraData: omitExtraData, | ||
}; | ||
const { comp, node } = createFormComponent(props); | ||
sandbox.stub(comp.ref.current, 'validateFormWithFormData').returns({ | ||
foo: '', | ||
}); | ||
fireEvent.submit(node); | ||
sinon.assert.calledWithMatch(comp.ref.current.validateFormWithFormData, formData); | ||
}); | ||
|
||
it('Should call validateFormWithFormData with a new formData with only used fields if omitExtraData is true', () => { | ||
const omitExtraData = true; | ||
const schema = { | ||
type: 'object', | ||
properties: { | ||
foo: { type: 'string' }, | ||
}, | ||
}; | ||
const formData = { foo: 'bar', baz: 'baz' }; | ||
const formRef = React.createRef(); | ||
const props = { | ||
ref: formRef, | ||
schema, | ||
formData, | ||
omitExtraData: omitExtraData, | ||
}; | ||
const { comp, node } = createFormComponent(props); | ||
sandbox.stub(comp.ref.current, 'validateFormWithFormData').returns({ | ||
foo: '', | ||
}); | ||
fireEvent.submit(node); | ||
sinon.assert.calledWithMatch(comp.ref.current.validateFormWithFormData, { foo: 'bar' }); | ||
}); | ||
}); | ||
|
||
describe('getUsedFormData', () => { | ||
it('should just return the single input form value', () => { | ||
const schema = { | ||
title: 'A single-field form', | ||
|
@@ -4352,6 +4403,58 @@ describe('Form omitExtraData and liveOmit', () => { | |
}); | ||
|
||
describe('validateForm()', () => { | ||
it('Should call validateFormWithFormData with the current formData if omitExtraData is false', () => { | ||
const omitExtraData = false; | ||
const schema = { | ||
type: 'object', | ||
properties: { | ||
foo: { type: 'string' }, | ||
}, | ||
}; | ||
const formData = { foo: 'bar', baz: 'baz' }; | ||
const formRef = React.createRef(); | ||
const props = { | ||
ref: formRef, | ||
schema, | ||
formData, | ||
omitExtraData: omitExtraData, | ||
}; | ||
const { comp } = createFormComponent(props); | ||
sandbox.stub(comp.ref.current, 'validateFormWithFormData').returns({ | ||
foo: '', | ||
}); | ||
act(() => { | ||
comp.ref.current.validateForm(); | ||
}); | ||
sinon.assert.calledWithMatch(comp.ref.current.validateFormWithFormData, formData); | ||
}); | ||
|
||
it('Should call validateFormWithFormData with a new formData with only used fields if omitExtraData is true', () => { | ||
const omitExtraData = true; | ||
const schema = { | ||
type: 'object', | ||
properties: { | ||
foo: { type: 'string' }, | ||
}, | ||
}; | ||
const formData = { foo: 'bar', baz: 'baz' }; | ||
const formRef = React.createRef(); | ||
const props = { | ||
ref: formRef, | ||
schema, | ||
formData, | ||
omitExtraData: omitExtraData, | ||
}; | ||
const { comp } = createFormComponent(props); | ||
sandbox.stub(comp.ref.current, 'validateFormWithFormData').returns({ | ||
foo: '', | ||
}); | ||
act(() => { | ||
comp.ref.current.validateForm(); | ||
}); | ||
sinon.assert.calledWithMatch(comp.ref.current.validateFormWithFormData, { foo: 'bar' }); | ||
}); | ||
|
||
it('Should update state when data updated from invalid to valid', () => { | ||
const ref = createRef(); | ||
const props = { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@helen-m-lin Since we released
5.18.5
, can you move this change comment up to the5.18.6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried rebasing your PR already and then I realized your comment needed to move. Plus another fix just merged so you'll have to rebase again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@heath-freenome I moved the changelog comment to
5.18.6
. Let me know if there is anything else!