Skip to content

Fixed schema type not being deduced correctly in compute defaults #1334#1338

Merged
epicfaace merged 1 commit intorjsf-team:masterfrom
t-moe:bug/defaultsAnyOf
Jul 9, 2019
Merged

Fixed schema type not being deduced correctly in compute defaults #1334#1338
epicfaace merged 1 commit intorjsf-team:masterfrom
t-moe:bug/defaultsAnyOf

Conversation

@t-moe
Copy link

@t-moe t-moe commented Jun 27, 2019

Reasons for making this change

As described in bug report #1334, the "default" values of schemas are not correctly applied to anyOf/oneOf properties.

This PR fixes that.
anyOf/oneOf schema don't have 'type' properties sometimes, so we needed to use getSchemaType(schema) instead of schema.type in computeDefaults

Checklist

  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed.

@epicfaace
Copy link
Member

@t-moe thanks! Can you add a test to make sure this bug doesn't regress?

@t-moe t-moe force-pushed the bug/defaultsAnyOf branch from c85a5cc to 96c21c5 Compare July 1, 2019 07:27
@t-moe
Copy link
Author

t-moe commented Jul 1, 2019

@epicfaace done

Copy link
Member

@epicfaace epicfaace left a comment

Choose a reason for hiding this comment

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

Can you rename those tests as I suggested? Thanks

@t-moe t-moe force-pushed the bug/defaultsAnyOf branch from 96c21c5 to fd1a675 Compare July 2, 2019 11:26
@t-moe t-moe force-pushed the bug/defaultsAnyOf branch from fd1a675 to 78e16db Compare July 3, 2019 05:54
@goralight
Copy link

Is there an Eta for this to be merged into master?

@epicfaace
Copy link
Member

Nothing pending, I just forgot to check on this. Thanks for the reminder @goralight !

@epicfaace epicfaace merged commit 750f5cb into rjsf-team:master Jul 9, 2019
@goralight
Copy link

goralight commented Jul 9, 2019

@epicfaace No worries! Thank you for merging and thank you @t-moe for fixing the bug. Could we expect a release soon ? :D

@MatinF
Copy link

MatinF commented Aug 21, 2019

@epicfaace As mentioned, I'm not sure this is yet fully resolved in the latest commit. Would be great to hear your thoughts on this. It looks like the default values are rendered in the editor, but in some cases these default values are not actually part of the submitted formData.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants